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

Copter & Plane: Report battery failsafe via the sys_status message #5574

Merged
merged 3 commits into from Jan 25, 2017

Conversation

WickedShell
Copy link
Contributor

Adds a MAVLink enum for the SYS_STATUS_SENSOR_BATTERY field, reports as present if the AP_BattMonitor primary instance is healthy, reports enabled if either of the battery failsafes are enabled, reports healthy as long as battery failsafe hasn't been triggered.

This is meant to address the fact that there is 0 persistent indication to the GCS that the aircraft is in a low battery/failsafe state. If everyone is fine with it I can extend it to Copter/Rover. Long term we still need a proper failsafe reporting system, however this is an easy fix for the larger problem, and actually is very consistent with the current reporting mechanisms.

@rmackay9
Copy link
Contributor

It's a slight stretch of the intended usage if we're talking about the new enum representing the battery monitor but it's ok if we consider it's the battery itself. Basically I'm ok with it.

@@ -215,6 +219,10 @@ void Plane::update_sensor_status_flags(void)
control_sensors_enabled |= MAV_SYS_STATUS_LOGGING;
}

if (g.fs_batt_voltage != 0 || g.fs_batt_mah != 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isnt using >0 better here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure, 0 was the enable value, although on reflection I think the voltage one is a float so that should change to is_zero(). I'm not sure if there is any logical reason, but I'm pretty sure you could be looking at negative voltages if you had a negative multiplier.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On reflection we do it as if current < fs_batt_voltage so that doesn't matter. Will change to > 0

@WickedShell WickedShell changed the title Plane: Report battery failsafe via the sys_status message Copter & Plane: Report battery failsafe via the sys_status message Jan 20, 2017
@WickedShell WickedShell added this to the AC 3.5.0 milestone Jan 24, 2017
@WickedShell
Copy link
Contributor Author

Rover doesn't have a battery failsafe yet, so this is good to be merged now.

@rmackay9 rmackay9 merged commit 0fec4af into ArduPilot:master Jan 25, 2017
@rmackay9
Copy link
Contributor

Merged.
I wonder how the MP is going to display this. I suspect it might show the "Check battery" warning on the HUD the whole time the battery is not plugged in. I guess we can ask michael to change that if we know how we want it to be displayed..

@lvale
Copy link
Member

lvale commented Jan 25, 2017

@DonLakeFlyer Do you have any suggestions ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants