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

[3.0-RC1] osd_camera_uptilt should not affect AHI on Fixed Wings #6990

Closed
b14ckyy opened this issue May 18, 2021 · 26 comments
Closed

[3.0-RC1] osd_camera_uptilt should not affect AHI on Fixed Wings #6990

b14ckyy opened this issue May 18, 2021 · 26 comments
Labels

Comments

@b14ckyy
Copy link
Collaborator

b14ckyy commented May 18, 2021

Current Behavior

if osd_camera_uptilt is set to a value of != 0 then the AHI will reposition up or down relative to camera tilt. This should not happen as then it will not align with Crosshair and Sidebar level markers anymore when we fly level.

Steps to Reproduce

  1. Enable AHI and Crosshair or Sidebars
  2. Level the craft to be at 0° pitch
  3. set osd_camera_uptilt=-20 to set a camera down tilt of 20°
  4. Result: Horizon will move 20° up relative to the crosshair

Expected behavior

The AHI should always align with Crosshair when craft is level.

Suggested solution(s)

make AHI independant from osd_camera_uptilt

Additional context


  • FC Board name and vendor: matek F411-WSE
  • INAV version string: INAV/MATEKF411SE 3.0.0 May 15 2021 / 17:01:46 (7b6a37a)
@b14ckyy
Copy link
Collaborator Author

b14ckyy commented May 18, 2021

Might have something to do with the changes made in #6896

@avsaase
Copy link
Member

avsaase commented May 18, 2021

WAI per #6616, although I did not really consider the crosshairs since I never use them.

@b14ckyy
Copy link
Collaborator Author

b14ckyy commented May 18, 2021

@avsaase
as discussed in messenger.
the osd_camera_uptilt is needed to align HUD elements in the OSD with the camera image. So it is needed for things like Waypoints, Radar elements or Home Point. But it should NOT affect the AHI then as the AHI has to be aligned with the crosshair on fixed wings for orientation.

Instead I recommend to use osd_ahi_vertical_offset instead. This value is currently used to modify the vertical position of the Pixel OSD AHI. the default should be set to 0 (I don't understand why default is -18 anyway) and then you can use the same value for realign the classic OSD AHI. This would also be much more consistent.

@breadoven
Copy link
Collaborator

Might have something to do with the changes made in #6896

#6896 just changes how much of the sidebar element is drawn. It has no affect on the position of the horizon arrows. If it's set to the default of 3 then it's just doing what it was doing before when it used a fixed constant value set at 3 (constant is still used for the Pixel OSD).

@avsaase
Copy link
Member

avsaase commented May 21, 2021

My thoughts on this:

  • To align the AHI with the actual horizon, it is better to specify the camera angle than the position on the OSD. Shifting the AHI up and down a few lines may work on a plane where the camera doesn't point down more than a few degrees, but on a quad, with 30 degrees camera uptilt the AHI moves completely out of view when pitching forward 30 degrees. This was the rationale behind Camera uptilt AHI compensation #6616. It is also much easier to specify the angle based on how you set up the camera than guess the number of rows. osd_ahi_vertical_offset is just the pixel OSD version of osd_horizon_offset so it should not be used for this purpose.
  • In my opinion, and other people may disagree, having the AHI consistently below the horizon is just plain wrong. It is an artificial horizon indicator, so it should at least roughly align with the actual horizon. I know it is also used as a "level flight" (as in "constant altitude") indicator but that fundamentally not what it is.
  • Personally, I never use the crosshairs because I find they don't add much, but I understand it is consuming to have the crosshairs below the AHI while the plain is not pitching down. However, I don't see this as an argument to have the AHI in the middle of the OSD when the camera looks down a few degrees but instead, the crosshairs should move up to match the AHI. After all, crosshairs should indicate where you are flying towards. But when the crosshairs are in the middle of the OSD then this is not the case and you always fly over the position they point to.
  • However, on a quad, the crosshairs should be in the middle of the OSD because that's the actual direction you fly in.

That brings us to the question of how to do all these things. One way would be to link the crosshair position to osd_camera_uptilt, but only on planes. When your camera looks down 5 degrees, this moves the crosshairs and the AHI up a little. This would be the most consistent solution but I understand some may want the crosshairs in the middle of the screen. This would only work for shallow camera angles, otherwise, the crosshairs would move out of view altogether. Alternatively, we can leave it like it is and tell people to move the crosshairs to align it with the AHI.

@DzikuVx, @OlivierC-FR WDYT?

@b14ckyy
Copy link
Collaborator Author

b14ckyy commented May 21, 2021

I understand your points regarding Quads. And there it is totally fine.

The main point is not the way how we move the AHI. The main concern I have is, that you hijacked the osd_camera_uptilt value to align the AHI instead of using the osd_ahi_vertical_offset, that IS actually meant to do exactly what you want. It was just only applied for Pixel OSD and not for classic one.

osd_camera_uptilt is used to align HUD elements (Waypoints, Home position, Radar, etc.) with the camera image. Take the osd_ahi_vertical_offset value to adjust the horizon on the classic OSD and all is fine.

The Crosshair offset (that is also a reference point for the AHI for obvious reasons) has to stay separated from pixel OSD as we can only shift rows on classic one but pixel precise on Pixel OSD.

@avsaase
Copy link
Member

avsaase commented May 21, 2021

osd_ahi_vertical_offset is not set in degrees but in pixels, so it cannot be used in the same way as I reused osd_camera_uptilt. And apart from that, doesn't it make sense to tie the AHI and HUD together? You point the camera down more -> all hud elements and the ahi move up. Both relate to physical things in view of the camera (home, other aircraft, horizon), so I don't see why they should behave differently.

@b14ckyy
Copy link
Collaborator Author

b14ckyy commented May 21, 2021

And apart from that, doesn't it make sense to tie the AHI and HUD together? You point the camera down more -> all hud elements and the ahi move up.

Yes. On Quads. But not on planes. It makes absolutely no sense on airplanes to rip off the AHI from the crosshair. It gets useless as you have seen in my video. The Crosshair on the plane is not there to show you the center of the camera but to show you the vector the nose is pointing at.

@b14ckyy
Copy link
Collaborator Author

b14ckyy commented May 21, 2021

Example. There is NO reference for me if the plane is flying level now.
If I adjust the Crosshair up by 2 rows then the AHI raises out of view. So i have to set the offset to 0 but then OSD elements won't match anymore.

I wanna use the Crosshair for proximity to target at a gap. This is impossible now without messing with the HUD elements too if I want to use AHI.
image

@OlivierC-FR
Copy link
Contributor

OlivierC-FR commented May 21, 2021

Interesting how seemingly simple things can get so complicated afterall :)

For now I have 3.0 only on my 5" quad, I fly this one exclusively in acro mode, usually at semi-fast forward speeds, so I didn't notice anything different about the AHI, it's out of view anyway and I don't think that many people are even looking at it on acro quads. I keep the AHI on on quads only in case of emergencies, if lost in a cloud or FPV cam or something like that ?

On planes it's a whole different thing, it's quite usefull to align the AHI on the crosshair and then know you're flying level.

To align the AHI with the actual horizon, it is better to specify the camera angle than the position on the OSD. Shifting the AHI up and down a few lines may work on a plane where the camera doesn't point down more than a few degrees, but on a quad, with 30 degrees camera uptilt the AHI moves completely out of view when pitching forward 30 degrees. This was the rationale behind #6616. It is also much easier to specify the angle based on how you set up the camera than guess the number of rows. osd_ahi_vertical_offset is just the pixel OSD version of osd_horizon_offset so it should not be used for this purpose.

With this I agree 100%.

Personaly I don't find it weird that the AHI line does not align with the real horizon, be it on quad or planes, but I can understand why others would find it weird... a matter of habit as often.

But in the end, my humble opinion is that the AHI should not move because of _uptilt.
There's the horizon setting for that. If I recall I had it constrain between -2 and +2 (for the classic OSD ofc), so an improvement would be to make it larger, like -6/+6 ? Or even more, to cover out-of-screen quad angles ?
I added _uptilt to adjust the 3D markers of the hub relative to the camera angle/frame, using that to move the AHI away the crosshair is... err... unplanned ?

@b14ckyy
Copy link
Collaborator Author

b14ckyy commented May 21, 2021

If I recall I had it constrain between -2 and +2 (for the classic OSD ofc), so an improvement would be to make it larger, like -6/+6 ? Or even more, to cover out-of-screen quad angles ?

that's the crosshair offset. I see no reason why the crosshair should be so high or low. maybe 3 lines but not more. As said on quads the crosshair makes sense as a reference of cam center. But on planes it is used to see where the plane si pointing at and flying towards.

On planes I see absolutely no reason to have the AHI not aligned with the crosshair. That's why I say that this makes no sense on fixed wing at all. If I have a camera tilt, I move the crosshair up and the Horizon has to follow and still align on level flight.

@OlivierC-FR
Copy link
Contributor

OlivierC-FR commented May 21, 2021

To me the crosshair should be center of the OSD, always. As it's always been. At some point I added the horizon offset with a narrow range so people could optionally move it by a little, seemed sensible to me since classic OSD is either 13 (NTSC) or 16 (PAL) lines, so with PAL an even number of lines, makes sense to move it a bit depending of taste (And without looking at the code for the pixel, I guess that's the reason for the -18 default. 18 pixels height per line, PAL center is probably on the "lower odd line")

I had a look at the DVR from yesterday's flight, 5" quad with 3.0, setting with a +20° uptilt, and indeed the AHI lines up nicely (when the horizon bug is not trolling around) with the real horizon. To be honest I never made use of that during the flight, neither noticed.

Conflicted feelings :)

Capture d’écran 2021-05-21 144806

@b14ckyy b14ckyy changed the title [3.0-RC1] osd_camera_uptilt should not affect AHI [3.0-RC1] osd_camera_uptilt should not affect AHI on Fixed Wings May 21, 2021
@b14ckyy
Copy link
Collaborator Author

b14ckyy commented May 21, 2021

OK I have changed the title.
On fixed wings it should stay aligned with crosshair all the time. As I said on quads I get the point. But not on fixed wings with cam down tilt. In this case the Crosshair and AHI has to be aligned with the physical horizon in level flight and HUD elements have to be adjustable for cam down tilt. THis is not possible right now.

@OlivierC-FR
Copy link
Contributor

OlivierC-FR commented May 21, 2021

So in the end, the fix is just to apply osd_ahi_vertical_offset also to the AHI of the classic OSD, no ?
And make it an angle, not lines or pixels.
Defaulted to 0°.
If the user wants to have the AHI relative to the real horizon he can set it to -uptilt, or leave it at 0 to have it crosshair relative.

@breadoven
Copy link
Collaborator

breadoven commented May 21, 2021

Just make #6616 multirotor only since that's what it's aimed at. This will make osd_camera_uptilt act as it did before for planes with multirotor users free to decide what they prefer.

@OlivierC-FR
Copy link
Contributor

OlivierC-FR commented May 21, 2021

IMHO the simplest is :

  • Remove horizon_offset (also for the pixel OSD). Crosshair will be screen-centered, period.
  • Add a toggle option : AHI is relative to crosshair (Default, 0°) / AHI is relative to real horizon (then correct the AHI angle by -camera_uptilt)

@b14ckyy
Copy link
Collaborator Author

b14ckyy commented May 21, 2021

Remove horizon_offset (also for the pixel OSD). Crosshair will be screen-centered, period.
please no! This is important for 3/4 of my fleet to have the crosshair together with the AHI aligned with the horizon.

I agree with @breadoven as a Multirotor only change or

Add a toggle option : AHI is relative to crosshair (Default, 0°) / AHI is relative to real horizon (then correct the AHI angle by -camera_uptilt)

this is also an option I would agree on. So everyone can select how it behaves.

@avsaase
Copy link
Member

avsaase commented May 21, 2021

IMHO the simplest is :

  • Remove horizon_offset (also for the pixel OSD). Crosshair will be screen-centered, period.
  • Add a toggle option : AHI is relative to crosshair (Default, 0°) / AHI is relative to real horizon (then correct the AHI angle by -camera_uptilt)

I'll do this tonight or this weekend.

Now that we're at it, we should also tackle #7000 since it is related. Depending on how you look at it, either the current or the previous AHI behavior makes sense.

@b14ckyy
Copy link
Collaborator Author

b14ckyy commented May 21, 2021

Good idea. Pawel has no time to do the changes at the moment.
It is not that critical as We could still use board pitch align for rough level and autolevel for fine tune but it is a bit counter intuitive and not what the main idea of the autolevel was.

@breadoven
Copy link
Collaborator

How about this:
https://www.youtube.com/watch?v=gkyxpR233_k

You can set whatever pitch interval you want, works for the full range of pitch. When AHI centred pitch numbers get obscured by the crosshair if enabled.

Not tried in flight so can't say if it might be useful or just annoying ... I suspect the latter.

@avsaase
Copy link
Member

avsaase commented May 22, 2021

Okay, so the solution we would all agree on boils down to adding a setting that determines whether or not the AHI is affected by osd_camera_uptilt. Correct? I have no issue with the option to shift the crosshairs.

@b14ckyy
Copy link
Collaborator Author

b14ckyy commented May 22, 2021

yes I think that's a consense there.
So if you turn AHI relation with osd_camera_uptilt on, then you it aligns with HUD Items for quads and if it is off, it will stay on Crosshair and can be moved with crosshair row shift.

And Pixel OSD has it's own settings with pixel position anyway. That sounds good to me.

@b14ckyy
Copy link
Collaborator Author

b14ckyy commented May 22, 2021

How about this:
https://www.youtube.com/watch?v=gkyxpR233_k

You can set whatever pitch interval you want, works for the full range of pitch. When AHI centred pitch numbers get obscured by the crosshair if enabled.

Not tried in flight so can't say if it might be useful or just annoying ... I suspect the latter.

I somehow like that. But I suggest to reduce it to 20° steps instead of 10° as the default horizon max visible pitch is 20° anyway. But is it possible to make the Main horizon line full length and the other lines shorter? Only 2 dashes on each side of the number? maybe we don't even need the numbers at all. Might be too distracting.

@breadoven
Copy link
Collaborator

How about this:
https://www.youtube.com/watch?v=gkyxpR233_k
You can set whatever pitch interval you want, works for the full range of pitch. When AHI centred pitch numbers get obscured by the crosshair if enabled.
Not tried in flight so can't say if it might be useful or just annoying ... I suspect the latter.

I somehow like that. But I suggest to reduce it to 20° steps instead of 10° as the default horizon max visible pitch is 20° anyway. But is it possible to make the Main horizon line full length and the other lines shorter? Only 2 dashes on each side of the number? maybe we don't even need the numbers at all. Might be too distracting.

It can be set to any pitch step you want between 0 and 30, 0 disables it 1 degree just turns it into pretty much a fixed AHI in pitch (with scrolling pitch readout) with AHI roll level indication. It is possible to change the number of dashes used as required although it gets a bit messy given how it works. The numbers wouldn't be so bad if they were smaller, half the size would still be readable and could be setup with each number having 2 positions at the top and bottom of the character space to minimise how out of step it gets with the dashed lines. But then that's 20 new characters which again gets messy. Without the numbers you lose reference to what the AHI is telling you although as you suggest, if the 0 degree main line is a different length to the other increments then you have some reference as to where level is. I'm thinking maybe it might be better without the numbers but with up and down arrows instead. Might give that a try, simpler and perhaps just as useful. You've always got the pitch and roll numeric fields anyway if you want to know the actual values.

@b14ckyy
Copy link
Collaborator Author

b14ckyy commented May 22, 2021

sounds great to me. Maybe open a Feature request instead of hijacking this PR :D

@stale
Copy link

stale bot commented Jan 9, 2022

This issue / pull request has been automatically marked as stale because it has not had any activity in 60 days. The resources of the INAV team are limited, and so we are asking for your help.
This issue / pull request will be closed if no further activity occurs within two weeks.

@stale stale bot added the Inactive label Jan 9, 2022
@DzikuVx DzikuVx closed this as completed Feb 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants