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

UAVCAN Smart Battery Improvements #14198

Merged
merged 71 commits into from
Apr 7, 2020
Merged

Conversation

AlexKlimaj
Copy link
Member

@AlexKlimaj AlexKlimaj commented Feb 21, 2020

Describe problem solved by this pull request
This PR is a continuation of @dakejahl 's work to integrate a UAVCAN smart battery.

Describe your solution
Improved the UAVCAN battery sensor bridge driver. Un-deprecated BAT_SOURCE to use as a parameter to disable battery_status module when a UAVCAN battery is being used.

Test data / coverage
Flight testing has been done with prototype ARK BMS smart batteries with UAVCAN. Flight testing with a single and dual batteries have been done.
Dual Battery Flight Test
Single Battery 25 Minute Flight Test

image
image
image

Additional context
See the BMS Workgroup.

@dagar
Copy link
Member

dagar commented Feb 22, 2020

Looks like we need some system wide configuration to properly configure the smart battery scenario.
Things like skipping batery_status start, making the uavcan smart battery an arming requirement, etc

@AlexKlimaj
Copy link
Member Author

I just completed a 25 minute hover test. It ended with the battery cutting out which is all on my side configuring the fuel gauge.

https://review.px4.io/plot_app?log=807e8661-99b8-4253-a840-e627ff2c02b0

We also need to think about how to support multiple batteries in parallel with this driver.

@AlexKlimaj
Copy link
Member Author

Here is a 25 minute flight on a Pixhawk 4 with this driver.

https://review.px4.io/plot_app?log=f0221744-79fc-4473-be30-932dcd7dbc2e

@AlexKlimaj
Copy link
Member Author

@dagar Just got a short flight with two smart batteries. With commander and mavlink already setup to handle multiple batteries, this driver works with multiple uavcan packs well.

https://review.px4.io/plot_app?log=e981e85c-466d-4841-9eff-c4094cef5e57
image

@AlexKlimaj
Copy link
Member Author

@dagar , @dakejahl and I went through the battery parameters last night and found that the new battery parameters are only loaded if the battery_status module is started. For the time being, I un-depricated BAT_SOURCE and I'm using it to not start battery_status. BAT%D_SOURCE for each battery doesn't really make sense either.

@AlexKlimaj
Copy link
Member Author

@dagar Any thoughts?

julianoes and others added 11 commits March 31, 2020 14:14
This moves the handling of the BAT%d_SOURCE param inside of the battery
library. Users of the library now pass the source instead of the flag
whether to publish. The battery library then checks if the source is
selected using the param and publishes accordingly.

Since we removed the strange system_source flag, we now need to look at
all batteries in commander.
For current estimation - I think - it makes sense to sum them up.
@AlexKlimaj
Copy link
Member Author

@AlexKlimaj @dagar this will heavily conflict with #14490. Could you review my suggestion for what to do with the BAT_SOURCE param?

Merged in #14490 and resolved conflicts.

@AlexKlimaj
Copy link
Member Author

@julianoes Got out and flew this PR with your branch merged in today. Everything is looking good.
Here is the longest flight:
https://review.px4.io/plot_app?log=2df1a307-1adf-463a-9d95-97285343d6d2

Copy link
Contributor

@julianoes julianoes left a comment

Choose a reason for hiding this comment

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

I guess you have rebased on #14490, that's cool.

@AlexKlimaj AlexKlimaj requested a review from dagar April 3, 2020 15:38
@mrpollo
Copy link
Contributor

mrpollo commented Apr 6, 2020

@dagar @julianoes what's missing here? can we merge this in?

@dagar dagar merged commit d8c140b into PX4:master Apr 7, 2020
@AlexKlimaj AlexKlimaj deleted the uavcan_smart_battery branch April 7, 2020 01:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants