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

Allow the HUD & AHI to be offset from the OSD page #1540

Merged
merged 3 commits into from
Jul 14, 2022

Conversation

MrD-RC
Copy link
Collaborator

@MrD-RC MrD-RC commented Jun 4, 2022

This PR allows the offset of the HUD and AHI in Configurator. Here's a demo of it working.

One thing of note. To me is seems backwards that negative values move the HUD up and positive values move the HUD down. I may address this in another PR later. Initially I thought it would be positive moves the HUD up, and negative down. This is why the data-setting-invert-select = "true" parameter has been added to settings.js. It reverses the order of the select box. Which would have been needed if 2 was to be at the top and -2 at the bottom.

Requires testing in HDZero
In theory it should work fine. But I don't have the equipment to test it in practice.

If testing in HDZero. Please could you align an element with the vertical centre of the AHI/HUD, like I have with the flight mode in the demo video. This way, we can see if 0 on the offset is correct to start with. Maybe @geoffsim could take a look?

@geoffsim
Copy link
Contributor

geoffsim commented Jun 5, 2022

As the configurator change is to adjust an existing parameter usually set on the command line, there is no reason why this would not work in HD. The HUD center coordinates are calculated using [rows/2, cols/2] so line 9 (18/2) column 25 (50/2) is the HD coordinates for the crosshair, etc. Note that the HUD will appear slightly lower and off to the right from center as the OSD uses a zero-based co-ordinate system.

Regarding the sign of the adjustment values. The OSD row numbering is from top to bottom (0 to 17 in HD). As such, an adjustment of -2 is logically correct when moving the HUD up.

To remove any confusion with the coordinate system, it might be better to hide the adjustment value from the user by using text in the drop down, say "UP 1", "UP 2", "DEFAULT", "DOWN 1", "DOWN 2", and converting the selection to the actual adjustment value.

In conclusion, this PR seems to be part of a much bigger PR that I was considering where, rather than using offsets, you could specify the coordinates of the HUD and the size of it (currently hard coded).

@MrD-RC
Copy link
Collaborator Author

MrD-RC commented Jun 5, 2022

Thanks for the feedback @geoffsim. I have since gotten confirmation that the zero point of the AHI matches in both the preview and goggles in HDZero, which means that the rest should work fine. I was really just asking for confirmation as I have no way of testing this myself. But, you saying that the OSD coords are zero indexed makes it sound like the preview position may be wrong. IMHO the preview should match exactly what you see in the goggles. Otherwise it's not much use. One character out in alignment is too much. Especially when it can be accounted for in configurator.

As for the positive and negative values to move the HUD. I understand how it works. But why should an end user have to know how the OSD is drawn? Positive values should move the elements up, and negative move them down. It should be intuitive. Just changing the select box I'm not a fan of, as it won't match with the CLI. Especially when all it needs is a simple inversion in the firmware.

@DzikuVx DzikuVx added this to the 6.0 milestone Jun 6, 2022
@geoffsim
Copy link
Contributor

geoffsim commented Jun 7, 2022

Whilst I disagree with changing how the vertical adjustment value affects the HUD, a suitably commented explanation should be acceptable, ex.

void osdCrosshairPosition(uint8_t *x, uint8_t *y)
{
*x = osdDisplayPort->cols / 2;
*y = osdDisplayPort->rows / 2;
*y -= osdConfig()->horizon_offset; // +ve moves the position up, -ve down.
}

@MrD-RC
Copy link
Collaborator Author

MrD-RC commented Jul 13, 2022

Requires iNavFlight/inav#8226

@MrD-RC MrD-RC merged commit b841253 into master Jul 14, 2022
@MrD-RC MrD-RC deleted the MrD_Move-AHI-on-OSD-screen branch July 14, 2022 20:17
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.

3 participants