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: battery failsafe logged even when FS_BATT_ENABLE = 0 #7619

Closed
rmackay9 opened this issue Jan 29, 2018 · 9 comments
Closed

Copter: battery failsafe logged even when FS_BATT_ENABLE = 0 #7619

rmackay9 opened this issue Jan 29, 2018 · 9 comments
Labels

Comments

@rmackay9
Copy link
Contributor

It is possible (as reported by Iketh in this discussion) for the a battery failsafe ERR message (Error 6) to be recorded to the dataflash logs even when the FS_BATT_ENABLE parameter is zero. Within the code, the failsafe.battery flag is also set to true.

This is different from how other failsafes are handled.

There are perhaps two options:

  1. change the FS_BATT_ENABLE parameter description so that "0" is "Report Only". This at least makes it clear to the user that it's not possible to disable the failsafe.
  2. add a new option "5: Report Only" which is the equivalent of the current behaviour and change the parameter default from "0" to "5" (this will mean most users who have not set the parameter will see no change in behaviour). Change the "failsafe_battery_event" method so it exits immediately if the parameter is set to "0" (or perhaps if "0" it should also clear the battery failsafe flag if set).

https://github.com/ArduPilot/ardupilot/blob/master/ArduCopter/sensors.cpp#L207

@Pedals2Paddles
Copy link
Contributor

Isn't this correct behavior? The wiki explains that it will still generate the low battery alert messages and tone alarm if you have voltage and mAh values set. I think all it needs is to not send an error with the word failsafe in it.

@rmackay9
Copy link
Contributor Author

It's at least different from other failsafes. For example with the RC failsafe, if it is disabled, there's no reporting at all. With ADSB_Avoidance failsafe, we have separate disabled and report-only options.

@Pedals2Paddles
Copy link
Contributor

Indeed. But it is also doing exactly what the documentation says it will do. http://ardupilot.org/copter/docs/failsafe-battery.html

If anything, it should probably just not use the word failsafe in the messaging?

@drjackso
Copy link

There is an additional bug, where if FS battery low is triggered Pixhawk will not check the voltage next time it is armed. i.e. if FS_BATT_ENABLE and set true, then battery is charged without resetting Pixhawk, Pixhawk will not arm due to "Check Battery".

@Pedals2Paddles
Copy link
Contributor

There is an additional bug, where if FS battery low is triggered Pixhawk will not check the voltage next time it is armed. i.e. if FS_BATT_ENABLE and set true, then battery is charged without resetting Pixhawk, Pixhawk will not arm due to "Check Battery".

I don't think that's a bug, it's intentional. But I understand that it does get in the way i you swap flight batteries without powering down the flight controller. But that's probably pretty rare for someone to do. And it's also the only way to reset the battery to 100% MAH.

@Pedals2Paddles
Copy link
Contributor

This should take care of the problem. #7639

@WickedShell
Copy link
Contributor

So #7213 was supposed to resolve this but doesn't. I actually don't think it's wrong (and no one objected at the time), but lets bring it up real quick and decide if it's correct or not.

As it stands right now if you set the trigger levels for a failsafe, but don't define an action (and are not already in a higher level action), we treat this as a warning. We still trigger all the alarms about battery failsafe (log the event, set the notify flag, warn the gcs), but we don't preform the action. I think this is correct because it allows an operator to make decisions if they want based on the warning going off without the vehicle doing anything. The only vehicle that enables any warnings by default is copter with a 3S battery voltage warning (which it really shouldn't have, but that's legacy and now we can't change it).

TL;DR
Basically I think the behavior as it stands is actually correct because it allows it to be used as a warning action, but I'd like to check first. @rmackay9 any strong opinions?

@rmackay9
Copy link
Contributor Author

So basically the user can't turn off the warning unless they set the FS_BATT_VOLTAGE (or whatver it's called now) to a low number. I think this is OK. We don't get a lot of complaints about it... we could just "handle it with documentation" by adding something to the wiki.

@WickedShell
Copy link
Contributor

Alright, closing is as intended behavior then.

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

Successfully merging a pull request may close this issue.

4 participants