-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
{Sponsored by Intelliterra} Added MAV_CMD_DO_SET_EMERGENCY for manual emergency setting #2027
{Sponsored by Intelliterra} Added MAV_CMD_DO_SET_EMERGENCY for manual emergency setting #2027
Conversation
…set remote id emergency status
I'll have a think about this next week. A few thoughts though:
|
"7.7 Emergency Status Indicator: F3586 − 22 7.7.1.2 The system shall automatically assert the emergency This change is targeting specifically 7.7.1.1.
|
@friissoren Would appreciate your thoughts on this. Yours too @tridge |
This makes sense to me at least. This would be the first drone ID MAV_CMD, but I guess that is not a problem. Everything else drone ID is handled via messages. Just thinking out loud here. In principle, it might have been nice to be able to send a text string with that cmd in order to better explain the type of emergency (the drone ID standards allow an optional text string to be transmitted for this purpose). However:
Is there a need to have something in the documentation about this CMD? |
If this CMD does get merged, I think it would be beneficial to include it in documentation |
<param index="2">Empty</param> | ||
<param index="3">Empty</param> | ||
<param index="4">Empty</param> | ||
<param index="5">Empty</param> | ||
<param index="5">Empty</param> | ||
<param index="6">Empty</param> | ||
<param index="7">Empty</param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm tempted to do this, which reserves two values as default NaN and the others as default 0 (the default). It matters if we ever want to extend this. See https://mavlink.io/en/guide/define_xml_element.html#reserved
We're very random on this.
<param index="2">Empty</param> | |
<param index="3">Empty</param> | |
<param index="4">Empty</param> | |
<param index="5">Empty</param> | |
<param index="5">Empty</param> | |
<param index="6">Empty</param> | |
<param index="7">Empty</param> | |
<param index="2" reserved="true" default="NaN" /> | |
<param index="3" reserved="true" default="NaN" /> |
From a purely message sending basis this command works. I presume it would be sent to the flight stack, which would then populate the LOCATION message? The benefit of a command is that you get confirmation and resends. As you say you can't send text values - IMO in a real emergency you shouldn't be typing or selecting anything. That all said, I dislike the whole idea - quite a lot. Imagine you're flying and you lose all power to one motor. I also suspect that in most cases the system will be faster and better at spotting an emergency than the user, and at deciding when not to do so because there is a chance of recovery. So I have my doubts whether this should exist. I think this needs a dev call in PX4/ArduPilot for more discussion. |
I am trying to remember why the MOC text was formulated as it was (see the extract that @jnomikos added above). Maybe @gabrielcox remembers? The FAA rule only mentions in §89.305 that an indication of the emergency status must be present. There is a bit more information available on page 127 but nothing is stated in the actual rule text about where/how the emergency must be detected and notified. |
Hi @friissoren , |
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-maintainers-call-august-22-2023/33813/2 |
@hamishwillee , @friissoren : The use model is very similar to setting a transponder code to 7700. It alerts those monitoring that your flight is in an emergency state. ATC currently comprehends and an alarm is triggered for them to take whatever action is necessary. For the RID rule, a means to express a similar construct is in the rule (an "emergency status"). In the MOC, this requirement can be satisfied by EITHER of the following options:
So, in practice, a pilot should at least assert the emergency status on the conditions of 2(1) or 2(2), but that's not a hard requirement and there may be other cases where it would make sense for a pilot to assert the emergency status (like the aircraft is on fire, and there's a dry tinderbox of vegetation beneath). Although these are practical uses, these are not mandated uses. The benefit to having this function in PX4 is for those systems that my not have a practical reason to implement option 2 above and would rather any emergency declarations be in the hands of the pilot. I suspect some systems may opt to implement both 1 and 2 to provide more system flexibility based on mission profile. I did see in a thread that when a person's flight is in trouble, they would focus on flying the aircraft. Although this sentiment is well received and semi-correct, there can be more nuance than this. In manned aviation, there's a rule of "Aviate, Navigate, Communicate -- in that order, especially in an emergency". They reinforce this in all pilot training. That said, emergencies are still declared, warnings are made to people on the ground when appropriate, |
Hi @gabrielcox Thank you very much - appreciate the quick response. My initial reaction is "this is not manned aviation". The pilot is not on the vehicle, and the probability that they know something about an emergency state that the vehicle itself does not, such as it being on fire, are close to zero. As you say though, if you are watching the vehicle, perhaps you might choose to rely less on automation under some circumstances. The command design here looks fine but have asked for ArduPilot/PX4 feedback, since they have to implement it. They may have more questions. |
ArduPilot has implemented 7.7.1.1 (manual) and 7.7.1.2 (automatically) assert an emergency. E.g. ArduPilot emergency code In the case of PX4 starting with manual asserting, then implementing an automatic asserting of an emergency in a follow up PR would be the best way forward. In case via QGroundControl an emergency has been set, also the text type of the emergency will be set to emergency (See MAV_ODID_DESC_TYPE). In the Remote ID apps you can see that it has been set to emergency. See screenshot. Of course an emergency can be set in the location message: MAV_ODID_STATUS Looking to F3586 I don't see any specific reference that an emergency status should be set in both messages. Being the devils' advocate, why have this CMD, whereas PX4 or ArduPilot could also look to the message type of the SelfID message? And would there be really a need to have set both status variables to emergency or is one of both also sufficient? |
@BluemarkInnovations What I think you are saying is that OPEN_DRONE_ID_SELF_ID is intended for and operator to assert a manual emergency - is that right? A command is not needed if that is correct. That message could perhaps have been better named to make it clear the origin is an operator and that is more than just an ID. |
It's wasteful and can't leverage the already existing logic for mav command retries and acknowledgements
Ardupilot has not implemented manual assertion but Mission Planner also has a button for an emergency. My opinion would be to use a mav command if we want big red buttons in these GCSs or we remove the buttons and only implement automatic assertion (which is done already in both projects) |
@hamishwillee No, it is intended to give an optional description for the flight. Like "my flight". If a manual emergency is asserted, it will change the type to emergency and set the description to something that describes the emergency. (This is one of the options that OpenDroneID messages allow you to do. The other is changing the location status.)
I was not involved in defining these messages, but also the flight controller may set this SelfID message to emergency and describing what the issue is in the text field. |
True, but this implementation already exists and is implemented by various parts in the eco-system. And if a user would already set a SelfID message, it would waste the same bandwidth already in normal operation. If ArduPilot and PX4 community both think this is a good idea, we should proceed! In my personal opinion, I only see some minor advantages and more drawbacks, that it would not justify the effort.
Yes I agree both PX4 and ArduPilot forward/relay these SelfID messages to the Remote ID transmitter. So implementing this manual emergency assertion method, means basically forwarding these messages from the GCS to the Remote ID transmitter. |
I agree that if ground control stations (such as QGC) want to have emergency buttons, it would be a good idea to have a MAV_CMD for setting emergencies. It might also be easier to get RID compliance by including a manual emergency button. In a theoretical situation where PX4 or Ardupilot were to fail to automatically set the emergency status during an uncontrolled descent or loss of control of flight trajectory, the system would likely fail to be RID compliant. But with the option of declaring a manual emergency, perhaps that would provide assurance of maintaining RID compliance regardless. But that was just my thought process. I think that theoretical situation I just gave would be extremely rare, and perhaps not something even worth caring about. After reading the discussion here, I am on the fence about if this change would be worthwhile. Really depends if we want emergency buttons on ground control stations or not. |
Yes - that is the implication of the service docs: https://mavlink.io/en/services/opendroneid.html. I was actually thinking though that the flight stack could also monitor this, and use it to set the location to emergency - following more the model of the suggested command. Either way, I think the mechanism needs to be explicit in the MAVLink spec - its a command, its that the remote id sets this from the self-id, or its that the flight stack sets the location message emergency setting from the self id.
According to #2027 (comment) it is "mandated that it should be possible". The arguments presented so far don't convince me either, but we should make sure this does get resolved. @auturgy did you get feedback from the ArduPilot dev call on preferred implementations? Mine would be that the autopilot should set the Location emergency field based on message or command from QGC. I slightly prefer the command, because the ground station is the origin, and you might not have a reliable channel to the FC - commands are better for that. |
Co-authored-by: Hamish Willee <hamishwillee@gmail.com>
Discussed this in the MAV call last night:L
The upshot of this is that we'll follow the normal rules. If there is interest at flight stack level to support this command, and resource to do so, then it can go into development.xml (not common.xml). We'd merge it there, and then move to common.xml if a prototype exists that shows this works effectively. At this point I don't see that commitment to implement. @jnomikos were you planning providing an implementation in QGC and PX4? If not, we should close this. We can reopen or recreate if interest strengthens. Make sense? |
I already created a working implementation in PX4 and QGC, but if this is unnecessary, I think it might be best for it to go into development.xml. What do you think? As a note, I would need to update my implementations in PX4 and QGC to use the new name of the message "MAV_CMD_ODID_DO_SET_EMERGENCY" |
|
Thanks @jnomikos. I've added a note to the PX4 PR. Essentially if PX4 really want this and are willing to merge we're OK with this. However they need to make that decision. Yes we will need the message name changed before merging in those PRs, but you could wait for them to make a statement of support or otherwise. This can remain open and in common for the moment. |
This or something like this does need to be merged for compliance. There are a bunch of other ways to do it yes, but having an emergency message is simple and clean. We implemented something like this but we can use this name and provide testing data as needed. An automatic approach is possible but this message in px4/QGC is quick and easy makes everything compliant. I guess I’m not understanding why we aren’t using this message. |
As said earlier in this PR, I don't oppose this implementation, but there is already an existing implementation provides the same functionality. The advantages of the existing solution would be: a) no need to change ArduRemoteID firmware b) no need to change QGC. Only PX4 needs to support two messages:
Of course, you also have this PR merged in Mavlink, but still you need additional PRs in QGC and PX4. And you would deviate from the ArduPilot implementation, which could potentially lead to more complex RemoteID implementations in future. PX4 can decide whatever is the best solution, the purpose of this comment is to summarize what is already available and I think that PX4 should also take the EU requirements in consideration as they are active since Jan 1st 2024. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the absence of any signal from PX4 on this over some months, and the absence of a maintainer, I am approving this. I will merge if I can get similar approval from @auturgy and @julianoes.
At latest that would be mav call, but I think you can plan for this to go in.
I am convinced by the logic of @ryanjAA and @BluemarkInnovations that this is simple, and does not preclude PX4 setting the emergency status automatically in future. I am pretty sure it is a pointless feature, but it will get us through regulatory requirements.
Agreed @hamishwillee, at the very least it will work for the time being and then an automatic trigger can be made. |
@hamishwillee Want me to move this to development.xml rather than common.xml still? |
I'm not opinionated on this and don't disagree to get this in. |
It makes way more sense to have less to think about and have this be auto but in the absence of that in next ~21 days, we at least need a manual trigger. |
The fact we have 21 days left now and are forced down this path makes me furious. This should have been sorted out the right way 6 months ago when we talked about automating this. I wonder if this could be fixed in the next 21 days properly? |
@ryanjAA We discussed on the 20240228-Dev-Meeting. We're agreed to merge if the associated work is going to be done in PX4 and QGC. Are you taking care of that or know there are people committed to it (in which case who)? Note thought that I might need to create a new PR for this because I can't edit this one. |
I already made changes in QGC and PX4 to accommodate for this, but they use the old name MAV_CMD_DO_SET_EMERGENCY rather than MAV_CMD_ODID_SET_EMERGENCY. I would be willing to make changes in this PR as well as in PX4 and QGC to allow for this change. I remember you mentioning you wanted this to go into development.xml rather than common.xml. I could do that, but if you want to make a new PR and do it instead, that works too. |
Closing, as the functionality merged in #2086 |
Added a new MAV_CMD called MAV_CMD_DO_SET_EMERGENCY.
The purpose of this command is for operators to be able to manually set their Remote ID status to emergency from a GCS (this status is emitted from the OPEN_DRONE_ID_LOCATION message).
An example of where this can be used is on QGroundControl. A "Declare Emergency" button exists for Remote ID but does not work since OPEN_DRONE_ID_LOCATION is handled on the PX4 side.
The PR that I am about to add on QGroundControl utilizes this command in the "Declare Emergency" button.
The PR that I am about to add on PX4 listens for this command and sets emergency status when it is received.
As a note, I tested this on QGroundControl by making the changes to the detached head submodule and building that first, since QGC currently is not on the most recent MAVLink build. I however did also build this one successfully.
Sponsored by Intelliterra