Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multirotor Althold throttle hover option + altitude adjustment indication #9220

Merged
merged 10 commits into from
Aug 23, 2023

Conversation

breadoven
Copy link
Collaborator

@breadoven breadoven commented Aug 6, 2023

Adds additional Hover throttle option for zero throttle position during Althold.

Also adds indication that altitude adjustment is active for multirotors, something that isn't always immediately obvious. Indication provided by "A" at start of altitude field. "A" alternates on/off if first field not blank. New system message also added but can be deleted if "A" prefix considered OK by itself.

@Jetrell
Copy link

Jetrell commented Aug 6, 2023

Would nav_mc_althold_throttle = Stick also assist in preventing the MC jumping or dropping, when switching out of Poshold back to Angle or Acro ?

@breadoven
Copy link
Collaborator Author

Would nav_mc_althold_throttle = Stick also assist in preventing the MC jumping or dropping, when switching out of Poshold back to Angle or Acro ?

STICK is the same as having the current nav_use_midthr_for_althold set to OFF.

The problem I've found with setting the Althold zero throttle position to the current stick position (nav_use_midthr_for_althold set to OFF) is the zero point can drift up or down if you switch between different Althold modes when altitude adjustment is active. This is because it keeps resetting the zero position to what was the previous adjusting position. If you switch in to Acro at that point the throttle stick may be way off hover throttle and you get the jump/drop until you move the throttle back to hover. The HOVER option should prevent this because the zero point can't drift it will always be set to the hover throttle setting nav_mc_hover_thr. You'll only get a jump/drop here if you switch to Acro when altitude is being adjusted and the stick isn't at the hover throttle zero position. MID_STICK would be similar although I'd have thought you will always have a jump/drop problem again when you switch to Althold if your hover throttle position is some way off mid stick. If it's below mid stick, maybe only 1/3 stick (which seems quite common), you'll get a drop until the throttle is raised to the mid position ... which isn't helpful. I have to admit I haven't tried MID_STICK for this reason since my quads hover at 1/3 throttle.

@Jetrell
Copy link

Jetrell commented Aug 7, 2023

When using nav_use_midthr_for_althold = hover it worked well.
If I was to increase or decrease the throttle, so the MC would climb or descend in ANGLE mode (not hovering stationary). Then I switched to POSHOLD or ALTHOLD, the quad would instantly stabilize and hover, with the throttle stick at what ever position it was at before entering either mode.. This was considerably better than the current method, especially at the low throttle side.

Then if an altitude adjustment was made in those modes, either up or down. The Altitude indicator would flash black and white, until the throttle stick was re-positioned for the craft to hold altitude again.
It works very nicely.. And makes it much easier for the pilot to find the hover throttle point again, after an altitude adjustment.

I tested it with two quads. One was Analogue video and the other Digital.
The Analogue OSD obviously work better, with it flashing black and white... While the Digital system has more of a flashing delay, due to MSP OSD lag on the HD system side. Which seems to prioritize character data refresh, over flashing character warnings.. So it only flashes once every 2 seconds..
What would be cool for HD in this case. Would be having a red or yellow colored number set in the HD font library.

@breadoven
Copy link
Collaborator Author

@Jetrell I assume you meant you used nav_mc_althold_throttle = HOVER ?

If you're using the HOVER setting I'd have thought if you were changing altitude when you engaged Althold that the quad would continue to change altitude albeit at a slower rate compared to Acro. I guess if you're still within the Althold deadband then adjustment wouldn't be active so it would stop and enter a stable hover.

@Jetrell
Copy link

Jetrell commented Aug 7, 2023

I assume you meant you used nav_mc_althold_throttle = HOVER ?

Yes.. Sorry about that. Wrong setting name.

I assume it was in the deadband.. Even though I was making it climb/drop as fast as I was comfortable with in LOS testing. Because I didn't what to punch or cut the throttle too harshly.
But HOVER certainly seemed smoother in transition than STICK, which is equivalent to what I normally use.

@rts18
Copy link

rts18 commented Aug 13, 2023

Altitude modes appear to be less fiddly when commencing a hover, after multiple altitude changes are made by stick input.

Indication provided by changing OSD altitude display to blinking + inverted. Feel free to suggest a better way of doing this if this seems overkill.

The idea is good even though the blinking character does not work well with HD OSD.
It makes it more difficult to assess any change in altitude, because the altitude character spends much of its time in the off state, with a HD inverted characters not possible.
Perhaps having the altitude symbol blnking and not the numerical value would be more useful. Or a higher blinking rate.

@breadoven
Copy link
Collaborator Author

Indication provided by changing OSD altitude display to blinking + inverted. Feel free to suggest a better way of doing this if this seems overkill.

The idea is good even though the blinking character does not work well with HD OSD. It makes it more difficult to assess any change in altitude, because the altitude character spends much of its time in the off state, with a HD inverted characters not possible. Perhaps having the altitude symbol blnking and not the numerical value would be more useful. Or a higher blinking rate.

I thought this might cause problems in some cases. I guess the possibilities are:

  1. Flashing symbol - not sure how easy this is given the symbol is formatted with the numeric value with the blink set after
  2. Use a different symbol - not desirable given current symbol overload
  3. Add an "A" or some other symbol after the altitude - however this increases the field length
  4. Add a system message - might be the simplest given most of the modes using altitude adjust don't have any other associated messages

@MrD-RC
Copy link
Collaborator

MrD-RC commented Aug 13, 2023

We could implement a 4 page font, with pages 3 and 4 being alert colours. Similar to BetaFlight, but with only pages 1 and 2 as standard, and 3 and 4 as alert.

It could be optional, and pretty simple to do. If the osd_use_alert_characters = ON and blink is set, increment the page by 2. Just do that before the page bits are set and I believe it should work. I’m sure that I changed the page bit from a boolean to actually setting the first 2 bits as future-proofing.

The question is, do we want to? Is there a chance that we will need to go on to page 3 for standard OSD. For example, we could make things like the arrows and heading graph more accurate, with more divisions. Maybe even the AHI. If we feel like standard OSD could need page 3. Then this alerts are a no-go anyway.

@rts18
Copy link

rts18 commented Aug 13, 2023

  1. Add a system message - might be the simplest given most of the modes using altitude adjust don't have any
    other associated messages.

@breadoven I reckon this would be the simplest and most workable option of the 4, at least for now.

Is there a chance that we will need to go on to page 3 for standard OSD. For example, we could make things like the arrows and heading graph more accurate, with more divisions. Maybe even the AHI. If we feel like standard OSD could need page 3. Then this alerts are a no-go anyway.

@MrD-RC I consider the Betaflight 3 page color warnings a bit of a waste. With every single symbol having three different colors.
I agree with your assessment of requiring more font page space in future expansion of the iNAV project. What if page 3 was left for this purpose. And page 4 was used for colored warning symbols?
Surely even one set of colored warning letters and numbers would be enough?

@MrD-RC
Copy link
Collaborator

MrD-RC commented Aug 13, 2023

That’s not how it works. The warning symbols need to match the position of the regular symbols, just on a different page.

Ideally, we could use a 6 page system. Which would give pages 1-3 for standard and 4-6 for warnings. But, that would break the MSP DisplayPort API.

@rts18
Copy link

rts18 commented Aug 13, 2023

That’s not how it works. The warning symbols need to match the position of the regular symbols, just on a different page.

Understood. That's what I was getting at. Only using orange for example, on page 4, in the ASCII letter and number character groupings.
I know its a waste of all the other characters on that page. But it would still provide HD users the option of having at least one color for warnings. While still leaving page 3 free.

@MrD-RC
Copy link
Collaborator

MrD-RC commented Aug 13, 2023

It wouldn’t work like that. It wouldn’t need to be a simple replacement for blink. So all symbols would need to be replicated. Otherwise there would need to be a hell of a lot of work to separate the symbols from alphanumerics, plus lookup tables. It would be an even bigger mess than the hack that had been needed for DJI O3 support.

@breadoven
Copy link
Collaborator Author

The other option to indicate adjustment is active is to show an "A" at the altitude field first character placeholder, where the "-" sign goes when 3 altitude numeric values are shown. The "A" is set to alternate on/off with around a 600ms period. Most of the time the -ve sign placeholder isn't used since altitudes are typically +ve and if altitude is negative then the -ve value is still obvious when the "A" alternates to off. This should also work with HD displays because it's not using the usual BLINK method.

@Jetrell
Copy link

Jetrell commented Aug 17, 2023

The other option to indicate adjustment is active is to show an "A" at the altitude field first character placeholder

I wasn't sure who you were forwarding the question to.
That idea sounds fine, and is more simplistic than a system message. But it may be too subtle, and harder to grasp what its telling people, without previously knowing what the 'A' stands for.. Does that makes sense ?

@breadoven
Copy link
Collaborator Author

The other option to indicate adjustment is active is to show an "A" at the altitude field first character placeholder

I wasn't sure who you were forwarding the question to. That idea sounds fine, and is more simplistic than a system message. But it may be too subtle, and harder to grasp what its telling people, without previously knowing what the 'A' stands for.. Does that makes sense ?

I think it should be possible to work out what it means intuitively but it can also be mentioned in the documentation. I'll add it to this PR ... see what people think especially for the HD systems.

@breadoven
Copy link
Collaborator Author

breadoven commented Aug 17, 2023

Added an "A" in the first position of the Altitude field to indicate altitude adjustment is active. The "A" is constant for altitudes above -99m and alternates on/off on 600ms cycle for altitudes below so the -ve sign isn't completely obscured. If this works for HD then it's a better solution than the system message which isn't very subtle especially when it's flashing on/off.

Edit: realised it needs fixing to also work with Imperial units.

@Jetrell
Copy link

Jetrell commented Aug 21, 2023

@breadoven I tested it again today. And I really like how much easier it is to engage the altitude holding point, after several changes to elevation are made with the throttle. You no longer have to hunt around with the stick to find that position.

Concerning the message and symbol on the OSD.
After flying with these, I find the System Message more intuitive and easier to notice.. No offense, but the "A" just seems like a misplaced character..
How about using OSD characters 342-343. (graphical vario) And use the up arrow to show an up altitude change, and the down arrow, to show a down altitude change ? These arrows are also colored on the HD OSD. So they'd be more noticeable.

Capture

@breadoven
Copy link
Collaborator Author

Concerning the message and symbol on the OSD. After flying with these, I find the System Message more intuitive and easier to notice.. No offense, but the "A" just seems like a misplaced character.. How about using OSD characters 342-343. (graphical vario) And use the up arrow to show an up altitude change, and the down arrow, to show a down altitude change ? These arrows are also colored on the HD OSD. So they'd be more noticeable.

Not so easy to do. Using characters > 255 is more awkward since they can't used with buff[] directly, but there are other arrow indicators < 255 that could be used. But then you'd need to check the direction of the adjustment which isAdjustingAltitude doesn't tell you. Can be done but starts getting a bit messy. Also the arrow indicator wouldn't be any more obvious than the "A" would it ? The "A" could be formatted so it's always right before the first number, would be easier to do, and shouldn't be confused as a misplaced character in that case surely ? Or just use the system message. I just thought it gets irritating when it's flashing on and off if your making a lot of small adjustments. Symbol 454 (Pan servo is centred) would be good to indicate adjustment, but > 255 again.

@Jetrell
Copy link

Jetrell commented Aug 21, 2023

Or just use the system message. I just thought it gets irritating when it's flashing on and off if your making a lot of small adjustments. Symbol 454 (Pan servo is centred) would be good to indicate adjustment, but > 255 again.

I'm happy enough with just the System Message.
But maybe a descriptive symbol relevant to the function could be added to the fonts ? There are still many empty character spaces below 255.

@breadoven
Copy link
Collaborator Author

I'll look at reusing symbol 454 for adjustment instead of an "A". Doesn't seem worth adding another symbol just for this and 454 is pretty much what you'd expect for an indication of adjustment.

@Jetrell
Copy link

Jetrell commented Aug 21, 2023

I'll look at reusing symbol 454 for adjustment instead of an "A". Doesn't seem worth adding another symbol just for this and 454 is pretty much what you'd expect for an indication of adjustment.

It might not look right in HD.. The Sneaky fonts, which most people use. Have Pan written in that symbol.

Capture

How about something simple like this for analogue.. The non embedded HD fonts can easily catch up.

Capture

@breadoven
Copy link
Collaborator Author

There's symbol 251, SYM_TERRAIN_FOLLOWING, which almost looks like altitude adjustment. Doesn't appear to be used ?

@Jetrell
Copy link

Jetrell commented Aug 21, 2023

Its not used yet.
This is it in the Sneaky HD fonts.. Shame its a plane and not a quad.

Capture

@breadoven
Copy link
Collaborator Author

Ah well, just leave it as a system message then for now. If interested someone can change it later in line with the "A" method but using an appropriate new symbol.

@breadoven
Copy link
Collaborator Author

LOL, changed my mind about the "A". Changed it so the "A" now sits next to the first altitude number so it's impossible to confuse it with an errant character. I think this is the simplest way of indicating that altitude adjustment is active which is the only purpose it serves after all.

@rts18
Copy link

rts18 commented Aug 22, 2023

There's symbol 251, SYM_TERRAIN_FOLLOWING, which almost looks like altitude adjustment. Doesn't appear to be used ?

@breadoven I was going to suggest using this symbol. Its glyph design in the iNAV embedded HD and analogue fonts are well suited.
I'm not a fan of the 'A'.
Sorry to say; it looks like an after thought.

This is it in the Sneaky HD fonts.. Shame its a plane and not a quad.

@Jetrell That image could easily be changed from a fixed wing to a multirotor in the SneakyFPV fonts.

@breadoven
Copy link
Collaborator Author

There's symbol 251, SYM_TERRAIN_FOLLOWING, which almost looks like altitude adjustment. Doesn't appear to be used ?

@breadoven I was going to suggest using this symbol. Its glyph design in the iNAV embedded HD and analogue fonts are well suited. I'm not a fan of the 'A'. Sorry to say; it looks like an after thought.

This is it in the Sneaky HD fonts.. Shame its a plane and not a quad.

@Jetrell That image could easily be changed from a fixed wing to a multirotor in the SneakyFPV fonts.

It is a bit of an afterthought I admit. Adding fonts is just a bit of a hassle which hardly seems worth it for this niche usage is my lazy reasoning.

@breadoven
Copy link
Collaborator Author

breadoven commented Aug 22, 2023

Changed to use SYM_TERRAIN_FOLLOWING instead of an "A" and system message deleted. If someone wants to add a better symbol then it's simple enough to change it as required. Should be OK to merge now.

@Jetrell
Copy link

Jetrell commented Aug 22, 2023

I'll inform Shannon (Sneaky font creator) of the change, so he knows what that symbol is now used for.

@breadoven
Copy link
Collaborator Author

breadoven commented Aug 23, 2023

I'll inform Shannon (Sneaky font creator) of the change, so he knows what that symbol is now used for.

SYM_TERRAIN_FOLLOWING (the INAV versions at least) would be better with a double arrow when used to indicate Alt Adjust but maybe it's better to just come up with a general use symbol indicating "adjustment" (something like symbol 454). Not sure what other use it might have though.

@breadoven breadoven merged commit 74320cb into iNavFlight:master Aug 23, 2023
15 checks passed
@Jetrell
Copy link

Jetrell commented Aug 23, 2023

Here is a modified 12 x 18 analogue character of 251 in the zip. So it looks like the HD versions. If you want to add it to the Configurator.. At least with this change you won't have to add a new character, it will just be an upgrade of the existing one.

Capture

251.zip

@breadoven breadoven added this to the 7.0 milestone Aug 23, 2023
@Jetrell
Copy link

Jetrell commented Aug 24, 2023

@breadoven Can I get you to take a look at the OSD altitude character set.. The whole thing, both the altitude number and the altitude symbol are not appearing on the screen, for both analogue and HD.

@breadoven breadoven deleted the abo_mc_althold_throttle_type branch August 25, 2023 07:29
@breadoven
Copy link
Collaborator Author

@Jetrell should work OK now hopefully.

@Jetrell
Copy link

Jetrell commented Aug 25, 2023

Thanks for the quick fix.. It now works again.

DzikuVx added a commit that referenced this pull request Sep 5, 2023
DzikuVx added a commit that referenced this pull request Sep 6, 2023
Bump navConfig PG that was forgotten in #9220
@sensei-hacker
Copy link
Collaborator

I'm not seeing the Configurator change to match the new name of the setting.
Was that done and I'm missing it, or shall I update Configurator?

@sensei-hacker
Copy link
Collaborator

sensei-hacker commented Mar 23, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants