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

Fix possibility of deep discharge when battery inserted and USB connected #9278

Merged
merged 1 commit into from
Apr 16, 2018

Conversation

MaEtUgR
Copy link
Member

@MaEtUgR MaEtUgR commented Apr 10, 2018

The autopilot system shuts down automatically with a dangerously low battery level if the drone isn't currently flying. This prevents deep discharge when forgetting to turn it off or unplug the battery. This feature currently doesn't work when the board is simultaneously connected via USB. Benchtesting sometimes requires usb connection and connected battery (of course propellers removed for safety) and if you then forget to look at the battery level it goes to deep discharge voltages (happened to me while I was in the last PX4 dev call).

The removed check is according to our understanding not needed (anymore) because the battery level is anyways only updated if there is a connected battery detected. See: https://github.com/PX4/Firmware/blob/master/src/modules/systemlib/battery.cpp#L207


commander: Remove usb_ connected flag on battery warnings
Battery warning gets only fired if a battery is inserted.

@MaEtUgR MaEtUgR added the bug label Apr 10, 2018
@MaEtUgR MaEtUgR self-assigned this Apr 10, 2018
@MaEtUgR MaEtUgR force-pushed the pr-prevent-deep-discharge-with-usb branch from fd0ce1f to 98c240f Compare April 11, 2018 09:19
julianoes
julianoes previously approved these changes Apr 12, 2018
mhkabir
mhkabir previously approved these changes Apr 13, 2018
@mhkabir mhkabir dismissed stale reviews from julianoes and themself via c52fefb April 15, 2018 04:37
@mhkabir mhkabir force-pushed the pr-prevent-deep-discharge-with-usb branch from d4d3eb5 to c52fefb Compare April 15, 2018 04:37
Battery warning gets only fired if a battery is inserted.
@MaEtUgR MaEtUgR force-pushed the pr-prevent-deep-discharge-with-usb branch from c52fefb to 543479e Compare April 16, 2018 14:29
@MaEtUgR
Copy link
Member Author

MaEtUgR commented Apr 16, 2018

Any review for the rebase? Nothing changed since the beginning.

@dagar
Copy link
Member

dagar commented Apr 16, 2018

I think this makes sense in the context of a real vehicle, but I'm wondering if it was added to prevent triggering these when powered by USB only?

@dagar
Copy link
Member

dagar commented Apr 16, 2018

Nevermind, I went through it again.

@dagar
Copy link
Member

dagar commented Apr 16, 2018

A bench test of the USB only case still wouldn't hurt.

@mhkabir
Copy link
Member

mhkabir commented Apr 16, 2018

Tested, works fine! Merging.

@mhkabir mhkabir merged commit 8a7dc1b into master Apr 16, 2018
@mhkabir mhkabir deleted the pr-prevent-deep-discharge-with-usb branch April 16, 2018 18:33
@hamishwillee
Copy link
Contributor

@MaEtUgR Would I be correct in thinking that this has no documentation impact (following our recent battery calibration updates)?

@mhkabir
Copy link
Member

mhkabir commented Apr 16, 2018

@hamishwillee correct :)

@MaEtUgR
Copy link
Member Author

MaEtUgR commented Apr 17, 2018

@hamishwillee Yes, it is expected behaviour and was just a bug.

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 this pull request may close these issues.

5 participants