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

msp_osd: reuse existing mode name conversion #22514

Merged
merged 1 commit into from
Dec 11, 2023
Merged

Conversation

MaEtUgR
Copy link
Member

@MaEtUgR MaEtUgR commented Dec 8, 2023

Solved Problem

When having a first look at the OSD code I found that all the modes names are explicitly defined again. This is also done in the corssfire driver and analog OSD for a long time already. I might fix those when I have things ready to test next.

Solution

Reuse the existing mode utility nav_state_names[] string definitions.
This saves a bit of flash, keeps the mode names up to date and works like expected.

Changelog Entry

Improvement: MSP OSD mode names come from common list

Test coverage

I finally assembled the hardware to fly and test the digital FPV system. Here are potato pictures of me switching through the modes. It works great, still only shows the 3 first letters but we can improve that later once I understand all the limitations.
IMG_20231208_213117
IMG_20231208_213123
IMG_20231208_213132
IMG_20231208_213138
IMG_20231208_213059
IMG_20231208_213105
IMG_20231208_213110

This saves a bit of flash, keeps the mode names up to date and
works like expected.
@MaEtUgR MaEtUgR self-assigned this Dec 8, 2023
@MaEtUgR
Copy link
Member Author

MaEtUgR commented Dec 11, 2023

Got a review from the author 🎉
image

@MaEtUgR MaEtUgR merged commit ece60b6 into main Dec 11, 2023
89 of 90 checks passed
@MaEtUgR MaEtUgR deleted the msp-osd-mode-names branch December 11, 2023 16:29
@hendjoshsr71
Copy link
Member

hendjoshsr71 commented Dec 12, 2023

In general this is fine,

Do keep in mind UI elements should be clear at a glance. You are flying through this screen.

The reason for the short names is that for DJI googles (unrooted) the only way to display custom messages is via the craft_name, char[13] message.

Currently the flight mode gets 3 chars.
Hold : "Hol" should be "HLD"
Acro : "Acr" should be "ACR"
Mission : "Mis" doesn't mean a lot ...

For flight modes some of these could be displayed using a different message as long there are BetaFlight equivalents. That shows the full mode name.

Note all of the above limits go out the window when using newer protocols like msp-display port (not in px4 yet).

@MaEtUgR
Copy link
Member Author

MaEtUgR commented Dec 13, 2023

@hendjoshsr71 Thanks for your feedback! I will definitely look into ways to improve the digital OSD. I still need to figure some things out.

Do keep in mind UI elements should be clear at a glance.

Totally

only way to display custom messages is via the craft_name, char[13] message.

That's what I presumed but didn't know exactly.

Currently the flight mode gets 3 chars.

I saw that but I wasn't clear about why DSRM uses 4 characters after it's written DISARMED just above.

should be "HLD", should be "ACR"

We could do custom mappings for these modes but honestly I'd rather allow a 4th character to have Hold and Acro written out in that case. And if it's purely about distinguishing abbreviated mode names even the first character already says everything for these.

"Mis" doesn't mean a lot ...

It's not ideal but before Return and Mission could not even be distinguished but instead both just showed as AUT.

For flight modes some of these could be displayed using a different message as long there are BetaFlight equivalents. That shows the full mode name.

So there are predefined ones in the MSP protocol that we could map to?

like msp-display port

Good point, maybe I'll just investigate that way.

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.

None yet

2 participants