PX4 / PX4-Autopilot

PX4 Autopilot Software
https://px4.io
BSD 3-Clause "New" or "Revised" License
8.17k stars 13.36k forks source link

add internal combustion monitor support #17861

Open PonomarevDA opened 3 years ago

PonomarevDA commented 3 years ago

Describe problem solved by the proposed feature We have a VTOL with internal combustion engine with controller (starter + throttle + ignition spark + fuel tank level sensor) based on UAVCAN. Although Autopilot may easily control it using RawCommand as well as any other motors, it doesn't have any features to monitor the status of ICE. In our case the critical information are RPM (it needs high rate, may be 5 Hz) and level of fuel tank (let's say at least once per 5 seconds).

Describe possible solutions We want to implement a modules which will forward uavcan messages such as uavcan.equipment.ice.FuelTankStatus and uavcan.equipment.ice.reciprocating.Status into MAVLink to see them in QGC. Possible MAVLink message could be EFI_STATUS. But EFI_STATUS is not enough because at least it doesn't have any info about current level of fuel tank. It has only info about consumption. Should I try to change EFI_STATUS or may be it's better to add another one like FUEL_STATUS?

Initially I could start with implementation of forwarding uavcan ice_status to mavlink EFI_STATUS. In parallel with it we can decide about fuel tank. What do you think about it? Could this feature be interesting?

dagar commented 3 years ago

Could this feature be interesting?

Yes!

If you know exactly what you then consider opening a Mavlink PR https://github.com/mavlink/mavlink, otherwise I'd suggest you start with EFI_STATUS or even GENERATOR_STATUS and go from there. https://mavlink.io/en/messages/common.html#EFI_STATUS https://mavlink.io/en/messages/common.html#GENERATOR_STATUS

It's great to see this data over Mavlink, but where I personally think it gets more useful is having the data and control within PX4. Everything relevant should get into uORB and be logged automatically. We should also start thinking about tying in the engine health and fuel level to failsafes. If possible PX4 being able to start the engine on arming, stop on disarm, and perhaps even restart in air would be interesting.

hamishwillee commented 3 years ago

ArduPilot handled this case in MAVLink by sending the BATTERY_STATUS message - using it's capacity values and setting other values to zero. This is obviously not ideal from a MAVLink perspective, but was chosen because it meant that a lot of GCS cases like low "battery" failsafe were supported without having to add any/much code. I can see the attraction of this, though I would have hoped perhaps for at least a MAV_BATTERY_TYPE to be added for fuel tank so that warning messages could be made more appropriate. From a MAVLink perspective ArduPilot are not interested in discussing this until they know that PX4 is definitely not following the same path (i.e. that we actually want a specific message).

@dagar Do you have a view on this? Further, looking at the proposed message is there anything you would add/remove: https://github.com/mavlink/mavlink/pull/1659

hamishwillee commented 3 years ago

ping @dagar ^^^

dagar commented 3 years ago

I think I could live with SYS_STATUS populating a battery_remaining percentage that's actual a fuel level, but extending that to BATTERY_STATUS seems a bit too far.

PonomarevDA commented 3 years ago

@dagar, may be I'm missing something, but SYS_STATUS on QGC doesn't process battery_remaining field, it processes only sensors. Also I see PX4 fills SYS_STATUS.battery_remaining with a lowest battery remaining, did I understand you correctly that you suggest to mix it with fuel tank? I think that fuel tank and battery are a bit different things with a bit different controlling purposes. For example, although low battery level should lead to landing or at least going home, for low fuel tank it's may be enough to just go the multicopter mode. So, at least a pilot controlling a VTOL most likely want to see separated data. Here, BATTERY_STATUS + FUEL_TANK looks pretty clear, but as @hamishwillee said BATTERY_STATUS might be enough. Looking a little bit out of this scope, somebody may need to have some payload for agriculture goals, for example to spread something on a field. And it also requires some control and at least own visualization. I mean, if we to put everything in a single BATTERY_STATUS message, a similar discussion might appear again in future.

@dagar, by the way, since you merged PR #17899, my next step could be doing the same with fuel tank. Here we have similar question. Should I use battery_status_s or may be it's better to create a separate uorb message?

dagar commented 3 years ago

@PonomarevDA it's not that I think SYS_STATUS battery_remaining is a good solution for fuel level, it's that I think it could be a less offensive short term hack to use for now until we figure out the real solution.