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

Advertise NAV modes through MSP even when mag is powered off (MR) #2984

Merged

Conversation

Projects
None yet
3 participants
@shellixyz
Copy link
Collaborator

commented Mar 29, 2018

Also checks compass health when arming if any navigation mode is enabled.

@shellixyz shellixyz force-pushed the shellixyz:show_nav_modes_when_no_mag_powered_off branch from 413cb2e to 822ba7e Mar 29, 2018

@digitalentity

This comment has been minimized.

Copy link
Member

commented Mar 29, 2018

Since we are looking at this, I suggest we change the way firmware advertises modes. Let's add a MSPv2 API that will also include a flag along with each box mode advertized to indicate if mode is currently supported by hardware or not. What do you think?

@shellixyz shellixyz force-pushed the shellixyz:show_nav_modes_when_no_mag_powered_off branch 2 times, most recently from 9c3a079 to d9ba100 Mar 29, 2018

@shellixyz

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 30, 2018

What do you have in mind that would require the MSP clients to know about all the modes even if they are not supported ? To show in the configurator for example the reason why a mode is not available ? I ask because the configurator is already not showing the modes not being supported by the hardware. I probably miss something.

@digitalentity

This comment has been minimized.

Copy link
Member

commented Mar 30, 2018

I'm thinking about situations when hardware is configured but failing. I.e. if compass is set to HMC5883 but was not detected at power up (hardware status - failing).

@digitalentity

This comment has been minimized.

Copy link
Member

commented Mar 30, 2018

Same should be done for GPS and baro

@@ -2485,7 +2487,9 @@ bool navigationBlockArming(void)
return false;

// Apply extra arming safety only if pilot has any of GPS modes configured
if ((isUsingNavigationModes() || failsafeMayRequireNavigationMode()) && !((posControl.flags.estPosStatue >= EST_USABLE) && STATE(GPS_FIX_HOME))) {
const bool compassIsHealthyOrNotRequiredNorConfigured = (STATE(FIXED_WING) && !compassIsConfigured()) || (getHwCompassStatus() == HW_SENSOR_OK);

This comment has been minimized.

Copy link
@digitalentity

digitalentity Mar 30, 2018

Member

Failing compass is an arming blocker by itself. I think this is not necessary

This comment has been minimized.

Copy link
@shellixyz

shellixyz Apr 2, 2018

Author Collaborator

I simplified the changes. The newly created compassIsConfigured() function is not really needed I'm using getHwCompassStatus() != HW_SENSOR_NONE instead.

Also you are right the checks in updateArmingStatus() should be enough. If the compass is configured and not calibrated nor healthy arming is prevented. I wasn't sure about that I should have checked. I removed the changes in navigation.c.

In the end the changes are very small.

@shellixyz shellixyz force-pushed the shellixyz:show_nav_modes_when_no_mag_powered_off branch 2 times, most recently from 8cd6609 to 82724cc Apr 2, 2018

@shellixyz

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 2, 2018

I'm thinking about situations when hardware is configured but failing. I.e. if compass is set to HMC5883 but was not detected at power up (hardware status - failing).

So if I understand correctly you would like the configurator to show all the modes but indicate for each unavailable mode the HW reason ?

@digitalentity digitalentity added this to the 2.0 milestone Apr 17, 2018

@shellixyz shellixyz force-pushed the shellixyz:show_nav_modes_when_no_mag_powered_off branch from 82724cc to 490519a Jun 18, 2018

@fiam fiam merged commit 98e3d0d into iNavFlight:development Jun 20, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.