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

Add fuel tank status support #23137

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Add fuel tank status support #23137

wants to merge 5 commits into from

Conversation

TSC21
Copy link
Member

@TSC21 TSC21 commented May 15, 2024

Solved Problem

Implemented support for fuel tank status reporting via MAVLink using the new FUEL_STATUS message.

Solution

  • Added support to DroneCAN FuelTankStatus messages
  • Added fuel_tank_status uORB message
  • Added FUEL_STATUS MAVLink stream
  • Added UAVCAN_ECU_MAXF parameter to define max fuel tank capacity

Note that mavlink/mavlink#2112 needs to be merged as well. This validated the implementation in mavlink/mavlink#2107.

Changelog Entry

For release notes:

New Feature: Added support for fuel tank status reporting via MAVLink.
New parameter: UAVCAN_ECU_MAXF
New paremeter: UAVCAN_SUB_FUEL

Alternatives

Data currently is fetched from DroneCAN, but can also be generated through an ECU driver in PX4.

Test coverage

None.

Context

@TSC21
Copy link
Member Author

TSC21 commented May 15, 2024

@dagar the build is going to fail for all the boards that are not setting development.xml as the default dialect (only px4_fmu-v5x and px4_sitl apparently set it). What should be the procedure here considering we are requested to implement new MAVLink messages on the flight stacks and by default new messages are added to development.xml? Am I missing a compile flag to disable the usage of this message when development is not used or is there a different way of doing this?

@DronecodeBot
Copy link

This pull request has been mentioned on Discussion Forum for PX4, Pixhawk, QGroundControl, MAVSDK, MAVLink. There might be relevant details there:

https://discuss.px4.io/t/px4-sync-q-a-may-15-2024/38773/1

@TSC21
Copy link
Member Author

TSC21 commented May 15, 2024

@dagar the build is going to fail for all the boards that are not setting development.xml as the default dialect (only px4_fmu-v5x and px4_sitl apparently set it). What should be the procedure here considering we are requested to implement new MAVLink messages on the flight stacks and by default new messages are added to development.xml? Am I missing a compile flag to disable the usage of this message when development is not used or is there a different way of doing this?

OK I have figured it out: 55eba3e

@TSC21 TSC21 force-pushed the add_fuel_status_support branch 3 times, most recently from dd24248 to 55eba3e Compare May 15, 2024 17:30
@dagar
Copy link
Member

dagar commented May 15, 2024

Looks good, I would just add the UAVCAN portion to kconfig so that we don't have to include this everywhere by default.

https://github.com/PX4/PX4-Autopilot/blob/55eba3e66a3418bcd457df5d6712d2f3122c0d63/src/drivers/uavcan/Kconfig

@TSC21
Copy link
Member Author

TSC21 commented May 15, 2024

Looks good, I would just add the UAVCAN portion to kconfig so that we don't have to include this everywhere by default.

https://github.com/PX4/PX4-Autopilot/blob/55eba3e66a3418bcd457df5d6712d2f3122c0d63/src/drivers/uavcan/Kconfig

Addressed. Thanks for the review!

@TSC21 TSC21 force-pushed the add_fuel_status_support branch 2 times, most recently from 0d197ac to e2fc35f Compare May 16, 2024 00:16
@hamishwillee
Copy link
Contributor

Just FYI we're still seeking comment on the new message, in particular around gas fuels.

TSC21 added 3 commits May 16, 2024 01:23
* Adds support to DroneCAN FuelTankStatus messages
* Adds fuel_tank_status uORB message
* Adds FUEL_STATUS MAVLink stream
* Adds parameter to define max fuel tank capacity
@TSC21
Copy link
Member Author

TSC21 commented May 16, 2024

Just FYI we're still seeking comment on the new message, in particular around gas fuels.

@hamishwillee can you clarify what that means for this PR? Because we don't have any source of gas fuel info in PX4 at this point, as FuelTankStatus is expected to come from an ICE, and the message doesn't provide any pressure information. It might definitely affect the fuel_tank_status uORB message, but nothing else in this pipeline.

@hamishwillee
Copy link
Contributor

@hamishwillee can you clarify what that means for this PR? Because we don't have any source of gas fuel info in PX4 at this point, as FuelTankStatus is expected to come from an ICE, and the message doesn't provide any pressure information. It might definitely affect the fuel_tank_status uORB message, but nothing else in this pipeline.

@TSC21 Very little. Really just a note that the message isn't live yet and is subject to change.

…d default values and handling of NaN and unknown cases
@TSC21 TSC21 requested a review from dagar May 16, 2024 11:59
@TSC21
Copy link
Member Author

TSC21 commented May 16, 2024

@dagar any suggestions on how to reduce the stack usage overall? Doesn't have to be in this PR, but we should follow-up on that. One thing that I saw that improves it is to use GCC 10 or above - apparently we are still using containers based from Ubuntu 20.04.

@TSC21
Copy link
Member Author

TSC21 commented May 20, 2024

@dagar any suggestions on how to reduce the stack usage overall? Doesn't have to be in this PR, but we should follow-up on that. One thing that I saw that improves it is to use GCC 10 or above - apparently we are still using containers based from Ubuntu 20.04.

Follow-up here: #23142.

@PonomarevDA
Copy link
Contributor

Oh, this is related to this PR: #18129.

Long time ago I tried to introduce FUEL_TANK message in MAVLink as well: mavlink/mavlink#1659, but it was not successful, so it was decided to use battery_status as a proxy.

Well, at least I can share my experience with dronecan fuel tank + PX4 v1.13.0 and my branch. I hope it will be useful. For our fuel sensor we use only remaining percent (this is the only thing that bettery_status allows). Other fields are just for logging.

Good things of using battery_status:

  • It can easily be displayed in QGC as a second battery and used by a pilot in a flight
  • It seems to be sufficient for a safety system: show a warning and trigger a failsafe on low level

But there are a few problems:

  • A pilot prefers to operate with a volume of the available fuel. A volume can't be displayed in QGC because battery_status doesn't have such a field.
  • It is not obvious to a pilot to differentiate what is actually a battery and what is fuel. Battery №10 is actually a battery because it has voltage, but battery №40 is fuel tank because it doesn't have.

So, I really like an idea to have a dedicated message. Additional fields like consumption rate look great as well. It would be nice to see in real-time a rate based on a flight mode.

Is there any discussion about how it will be implemented in QGC?

@TSC21
Copy link
Member Author

TSC21 commented May 28, 2024

So, I really like an idea to have a dedicated message. Additional fields like consumption rate look great as well. It would be nice to see in real-time a rate based on a flight mode.

Is there any discussion about how it will be implemented in QGC?

Not yet but there will eventually be a follow-up. Though, work in QGC should be independent from the one in PX4, as the interface is MAVLink - so anything happening in QGC will mostly depend on getting the MAVLink message merged and stable.

@hamishwillee
Copy link
Contributor

For our fuel sensor we use only remaining percent (this is the only thing that bettery_status allows). Other fields are just for logging.

THanks @PonomarevDA - I'd forgotten your message, but doubtless it seeded my thinking for the new one.

There are many problems with BATTERY_STATUS, which is why we are hoping to replace it with BATTERY_STATUS_V2. The main problem in this context is that it was designed around a power monitor, where the only value you know to be true is the consumed current. Everything else is guesswork based on the assumption that the battery was full when you started.

For the new fuel message we're making no assumptions other than the values you send are measured or must be guaranteed to be accurate. So if you measure only fuel consumption then you'd send tank capacity (known to be true) and fuel consumption, and the recipient GCS knows that the remaining fuel and percentage are guesses based on the assumption tank was originally full. It can then prompt for you to fill tank before flying. If the message supplies the remaining fuel, then you know it is actually measuring that, and you can calculate the percentage if you want from the total capacity - in this case you can warn the user to fill up, but you only need to do so when they are low on fuel, not in order to know how much fuel they have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

None yet

5 participants