mavlink / qgroundcontrol

Cross-platform ground control station for drones (Android, iOS, Mac OS, Linux, Windows)
http://qgroundcontrol.io
3.31k stars 3.63k forks source link

Disconnect doesn't really disconnect UAS #962

Closed DonLakeFlyer closed 9 years ago

DonLakeFlyer commented 10 years ago

UAS stays in the system until you go to Mission and use the Context menu to remove the UAS from the system. This can cause all sorts of problems, because the active UAS stays connected to all the UI elements.

For example you can do this:

QGC stills thinks it talking to the first board. That's a bit scary. An example with a single board:

Now any UI which is keyed off of mavtype will now be wrong because it hasn't noticed the as changing out from under it. Like for example HIL Config. So you have to reboot QGC to get HIL to give you new aircraft choices.

LorenzMeier commented 10 years ago

@DonLakeFlyer I've fixed a missing signal recently which didn't lead to a configuration change announcement as expected. Its now working correctly (airspeed sensor doesn't show up for multicopters any more).

LorenzMeier commented 9 years ago

@DonLakeFlyer Ping - this should be at least improved, if not fixed, on master.

DonLakeFlyer commented 9 years ago

I haven't seen any different behavior in master. The title isn't quite correct. The link itself does disconnect. But the UAS object itself still hangs around. Also QGC complains about losing the connection. It doesn't distinguish between the user disconnecting and a bad connection.

Paste from email: The whole connect/disconnect behavior as it relates to a UAS seems strange to me. Some things I don’t understand:

LorenzMeier commented 9 years ago

I would like to offer three main arguments on this. Note that the mindset is that you have potentially more than one link to a system. The autopilots sold in 2015 will have bluetooth, wifi and LTE and might still retain the radio modems, and we have to ensure to not bake the one airframe = one radio modem assumption into the application.

DonLakeFlyer commented 9 years ago

Ok, I can live with most of this. If the id re-use during the same session is uncommon then it’s fine.

I still don’t understand the multiple active links on a single UAS thing. I understand an autopilot could have multiple ways to connect to it. But what does it mean when a single UAS has two links associated with it, both connected at the same time? How does that even work? You would have two mavlink streams coming through to the UAS. Maybe a different component id on each stream. I don’t think the code can really handle that.

On Nov 24, 2014, at 1:05 PM, Lorenz Meier notifications@github.com wrote:

I would like to offer three main arguments on this. Note that the mindset is that you have potentially more than one link to a system. The autopilots sold in 2015 will have bluetooth, wifi and LTE and might still retain the radio modems, and we have to ensure to not bake the one airframe = one radio modem into the application.

It might make sense to avoid to link systems with links, the same way as databases on hosts are not linked to network cards - while a database without network often doesn't make sense, its architecturally independent. So the current architecture makes a lot of sense to me. If the user has multiple airframes and all have the same system ID, its his problem if he gets confused - we can try to help though, but not by making an airframe system with LTE and radio modem impossible. A link isn't a UAS-specific thing - its a network interface with 1..N systems connected. We already have and will even more so have multiple links to systems (radio, LTE, Wifi, Bluetooth). The only decent approach to solve this is to send the UUID of the system (MCU serial) and complain if the logic ID is used by multiple UUIDs. In reality on normal usage I don't see that this would happen very often. Disconnecting the link: For the same argument on network interfaces, it doesn't make sense to me to delete the system or disable the complaining animation that the system is missing once the link is disconnected. In a real application scenario an user finished with flying will close the application (or switch to the post-flight view, at which point he leaves the live workspace and we could prompt him wether he wants to remove existing connections and systems). If the user doesn't close the application or switch to post-flight, he probably wants to continue flying, and because he might have multiple links, we have to assume that if he wants to fly we should care about timed out systems. — Reply to this email directly or view it on GitHub https://github.com/mavlink/qgroundcontrol/issues/962#issuecomment-64264521.

LorenzMeier commented 9 years ago

We can add the UUID here as field or reuse some bytes of the custom ID: http://mavlink.org/messages/common#AUTOPILOT_VERSION

And we could send the message in this function call: https://github.com/PX4/Firmware/blob/master/src/modules/mavlink/mavlink_messages.cpp#L528

And pull the GIT version string from here: https://github.com/PX4/Firmware/blob/master/src/systemcmds/ver/ver.c#L102

LorenzMeier commented 9 years ago

The code has been built from day one for multiple streams, and we had it running just this summer in OBC. Its not so difficult as a model - its equivalent of sending and receiving messages on two UDP ports - that's not a problem on networking either.

DonLakeFlyer commented 9 years ago

Also just as FYI: There is a lot of UI code which is written in such a way that it assumes that if it has an active UAS, that UAS is connected. As far as I’ve seen, no UI code checks for the active UAS connection to be active. If the UAS is going to stay around while disconnected a lot of bug fixing is going to have to happen. Even if it is theoretically the right thing it would be way easier to toss the UAS when it has no connections to save a lot of code work.

On Nov 24, 2014, at 1:18 PM, Donald Gagne don@thegagnes.com wrote:

Ok, I can live with most of this. If the id re-use during the same session is uncommon then it’s fine.

I still don’t understand the multiple active links on a single UAS thing. I understand an autopilot could have multiple ways to connect to it. But what does it mean when a single UAS has two links associated with it, both connected at the same time? How does that even work? You would have two mavlink streams coming through to the UAS. Maybe a different component id on each stream. I don’t think the code can really handle that.

On Nov 24, 2014, at 1:05 PM, Lorenz Meier <notifications@github.com mailto:notifications@github.com> wrote:

I would like to offer three main arguments on this. Note that the mindset is that you have potentially more than one link to a system. The autopilots sold in 2015 will have bluetooth, wifi and LTE and might still retain the radio modems, and we have to ensure to not bake the one airframe = one radio modem into the application.

It might make sense to avoid to link systems with links, the same way as databases on hosts are not linked to network cards - while a database without network often doesn't make sense, its architecturally independent. So the current architecture makes a lot of sense to me. If the user has multiple airframes and all have the same system ID, its his problem if he gets confused - we can try to help though, but not by making an airframe system with LTE and radio modem impossible. A link isn't a UAS-specific thing - its a network interface with 1..N systems connected. We already have and will even more so have multiple links to systems (radio, LTE, Wifi, Bluetooth). The only decent approach to solve this is to send the UUID of the system (MCU serial) and complain if the logic ID is used by multiple UUIDs. In reality on normal usage I don't see that this would happen very often. Disconnecting the link: For the same argument on network interfaces, it doesn't make sense to me to delete the system or disable the complaining animation that the system is missing once the link is disconnected. In a real application scenario an user finished with flying will close the application (or switch to the post-flight view, at which point he leaves the live workspace and we could prompt him wether he wants to remove existing connections and systems). If the user doesn't close the application or switch to post-flight, he probably wants to continue flying, and because he might have multiple links, we have to assume that if he wants to fly we should care about timed out systems. — Reply to this email directly or view it on GitHub https://github.com/mavlink/qgroundcontrol/issues/962#issuecomment-64264521.

LorenzMeier commented 9 years ago

I'm not sure I'm following - as far as I know it will cycle through an empty list and ending up sending nothing and timing out. Sounds like we will want to offer switching to post-flight and lock the complete active UI. I have a hard time imagining a use case where you have no links but want to operate the flight UI actively.

DonLakeFlyer commented 9 years ago

Use QGCParamWidget as an example:

May be a bad example, but all the other UI works the same way. The user will think they are doing something, but it will either just not do it silently or other strange things would happen on a case by case basis.

On Nov 24, 2014, at 1:28 PM, Lorenz Meier notifications@github.com wrote:

I'm not sure I'm following - as far as I know it will cycle through an empty list and ending up sending nothing and timing out. Sounds like we will want to offer switching to post-flight and lock the complete active UI. I have a hard time imagining a use case where you have no links but want to operate the flight UI actively.

— Reply to this email directly or view it on GitHub https://github.com/mavlink/qgroundcontrol/issues/962#issuecomment-64267681.

DonLakeFlyer commented 9 years ago

Also missed your last sentence: If there is no use case for no links and operating the flight UI. Then why keep the UAS around? If you remove the UAS on the last link going away, then the UI will do the right thing (because it gets a setActiveUAS(NULL)) and disable itself in the right places instead of just acting strangely when it doesn’t have a connection.

On Nov 24, 2014, at 1:36 PM, Donald Gagne don@thegagnes.com wrote:

Use QGCParamWidget as an example:

  • Gets setActiveUAS when the first connection is made
  • UAS is then disconnected
  • QGCParamWidget gets no signal from that, it still has link to disconnected has
  • Then say the user clicks write to ROM
  • Calls uas->writeParametersToStorage
  • Which then tries to send a mavlink message
  • Other than a qDebug message that the link is not connected the user will see nothing and think that what was asked for happened

May be a bad example, but all the other UI works the same way. The user will think they are doing something, but it will either just not do it silently or other strange things would happen on a case by case basis.

On Nov 24, 2014, at 1:28 PM, Lorenz Meier <notifications@github.com mailto:notifications@github.com> wrote:

I'm not sure I'm following - as far as I know it will cycle through an empty list and ending up sending nothing and timing out. Sounds like we will want to offer switching to post-flight and lock the complete active UI. I have a hard time imagining a use case where you have no links but want to operate the flight UI actively.

— Reply to this email directly or view it on GitHub https://github.com/mavlink/qgroundcontrol/issues/962#issuecomment-64267681.

DonLakeFlyer commented 9 years ago

Probably should have asked: What can you do from an interaction standpoint with a disconnected UAS? I haven’y run across anything. But I don’t really know the full UI.

On Nov 24, 2014, at 1:39 PM, Donald Gagne don@thegagnes.com wrote:

Also missed your last sentence: If there is no use case for no links and operating the flight UI. Then why keep the UAS around? If you remove the UAS on the last link going away, then the UI will do the right thing (because it gets a setActiveUAS(NULL)) and disable itself in the right places instead of just acting strangely when it doesn’t have a connection.

On Nov 24, 2014, at 1:36 PM, Donald Gagne <don@thegagnes.com mailto:don@thegagnes.com> wrote:

Use QGCParamWidget as an example:

  • Gets setActiveUAS when the first connection is made
  • UAS is then disconnected
  • QGCParamWidget gets no signal from that, it still has link to disconnected has
  • Then say the user clicks write to ROM
  • Calls uas->writeParametersToStorage
  • Which then tries to send a mavlink message
  • Other than a qDebug message that the link is not connected the user will see nothing and think that what was asked for happened

May be a bad example, but all the other UI works the same way. The user will think they are doing something, but it will either just not do it silently or other strange things would happen on a case by case basis.

On Nov 24, 2014, at 1:28 PM, Lorenz Meier <notifications@github.com mailto:notifications@github.com> wrote:

I'm not sure I'm following - as far as I know it will cycle through an empty list and ending up sending nothing and timing out. Sounds like we will want to offer switching to post-flight and lock the complete active UI. I have a hard time imagining a use case where you have no links but want to operate the flight UI actively.

— Reply to this email directly or view it on GitHub https://github.com/mavlink/qgroundcontrol/issues/962#issuecomment-64267681.

DonLakeFlyer commented 9 years ago

To recap:

As far as problems with the UI keeping hold of a disconnected UAS and doing strange things:

Option 1 - assumes there is valid UI to show for a disconnected UAS:

Option 2 - assumes there is no valid UI to show for a disconnected UAS:

Both of these options require minor change to the codebase. I don't know whether 1 or 2 is appropriate, because I don't know the answer to the main assumption.

DonLakeFlyer commented 9 years ago

One last comment since I'm fighting with this right now: