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
AP_OSD: support for main rotor rpm #23711
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
main thing this needs is #if for RPM support
@@ -1638,6 +1654,22 @@ void AP_OSD_Screen::draw_heading(uint8_t x, uint8_t y) | |||
backend->write(x, y, false, "%3d%c", yaw, SYMBOL(SYM_DEGR)); | |||
} | |||
|
|||
void AP_OSD_Screen::draw_rrpm(uint8_t x, uint8_t y) | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to check for #if AP_RPM_ENABLED and include AP_RPM/AP_RPM_config.h
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Andrew thank you so much for the recommendations: I have pushed mods in the "fix" commit below, don't know if there's better way to do it though.
libraries/AP_OSD/AP_OSD_Screen.cpp
Outdated
// No RPM because pointer is null | ||
_rrpm = -1; | ||
} | ||
backend->write(x, y, false, "%4.0f%c", (double)_rrpm, SYMBOL(SYM_RPM)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is a little more efficient to cast to an integer than double
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The #if really should be around the variables and parameters too, so there is zero flash cost for boards without RPM, and it doesn't show parameters that can't be used
@@ -2251,6 +2290,7 @@ void AP_OSD_Screen::draw(void) | |||
DRAW_SETTING(heading); | |||
DRAW_SETTING(wind); | |||
DRAW_SETTING(home); | |||
DRAW_SETTING(rrpm); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm surprised this compiles with RPM disabled? It will call a non-existant function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good! just needs the commits squashed (I can do that if you are not familiar with it)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
displays main rotor rpm in OSD
Thank you! |
.... otherwise include chain on bootloaders can try to include mavlink
I've pushed up a patch to resolve the compilation problem this was having in CI |
displays main rotor rpm in OSD