mavlink / qgroundcontrol

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

We really shouldn't ask our users to manage UDP ports themselves #1681

Closed LorenzMeier closed 8 years ago

LorenzMeier commented 9 years ago

QGroundControl opened for the longest time a default connection on UDP port 14550. There is literally not a single reason to not do this and require the user to set it up manually, and it did not only break the AR.Drone support, but its also adding back an unnecessary bit of complexity for tablet users.

@dogmaphobic I'm assigning this to you since you did UDP hackery and auto-discovery. I think what we want is to have the link up and running by default, and some method of finding drones on the local network.

dogmaphobic commented 9 years ago

I'm not sure I follow. The default port (14550) has not changed. How was the AR.Drone support broken?

As to the auto discovery, the ultimate intent is to have an option for UPD to be always active and published. That way, whenever a client supporting auto discovery is started, it automatically connects to QGC.

dogmaphobic commented 9 years ago

Tablets will have UDP up and running automatically. Whenever a client (a Pixhawk with Zeroconf software) is activated, a pop up will show up asking if you want to connect. I have the same plans in place for Cellular connection. That one will require registering with a broker service, which I've also already started, for both sides finding each other in the wide (Internet) network. The goal is to not to have to configure anything. Just start and it all works.

LorenzMeier commented 9 years ago

We need the same for the desktop - that's all. I'll try to show you SITL in the next days, which should help a bunch with testing.

DonLakeFlyer commented 9 years ago

The key is to do it in such a way that there is not a connection hanging around on the connect dropdown that users will get confused with. Needs to work like plugging in pixhawk to USB: Only there when plugged in. Not there when not plugged in. That way people don't try picking it in the wrong circumstances. Probably need to sniff connections for mavlink.

dogmaphobic commented 9 years ago

Besides MavProxy and mavros, both of which requires a great deal of understanding to get it even working, what else connects to QGC using UDP? How is that setup (on the UAV side)?

DonLakeFlyer commented 9 years ago

what else connects to QGC using UDP

Other than AR.Drone I don't know, and I don't really know how that works from the vehicle side of things.

dogmaphobic commented 9 years ago

I can't say I had never heard of AR.Drone but until now, I had never paid any attention to it. I went to their web site and I'm not sure I understand what it is. I did notice they use QGC for controlling it (news to me). Does it have a Pixhawk in it? What exactly connects to QGC and how? Is it a normal QGC or some customized version?

dogmaphobic commented 9 years ago

While at it, I'm still banging my head trying to figure out why QGC stops receiving UDP datagrams after a while (the while varies from several minutes to seconds). I've found and fixed several minor issues while debugging this but the main problem remains. You open up an UDP connection with QGC and everything works fine for a while but then, it stops receiving readyRead() signals. The core code in Qt seems to be stuck on a select(). It can't recover from it. If you close the connection and open again, it goes back working (until it stops again at some point).

dogmaphobic commented 9 years ago

All right, fixed it by not using Qt's event loop (running my own). I will send a PR for that and then I will take a look at this one. Still not sure what connects to QGC and how though...

LorenzMeier commented 9 years ago

The de-facto standard is that the GCS is waiting for incoming packets on port 14550. The PX4 SITL does the same thing.

dogmaphobic commented 9 years ago

I understand the receiving end (QGC) just fine. What I'm trying to understand is the other end. How is AR.Drone connecting to QGC? How does it find QGC? I had never heard of this before today.

LorenzMeier commented 9 years ago

You have to ask Parrot - I just know what's needed to be done on the GCS side and that users where happy with the previous state. I presume the drone scans the network or something.

dogmaphobic commented 9 years ago

What I will do is to automatically add an UDP link using the default port. This will always be present. The user will still need to select "UDP Link" when pressing the connect button though. This at least avoids the need to configure a new link.

Regarding asking Parrot, I don't really care what they do. I was just trying to understand what they do so I could make it work better for them.

LorenzMeier commented 9 years ago

That doesn't sound like a fix. You're not clicking in your mail client connect before you can send an email, do you?

Try to focus on the user. For the case of an UDP link, why the heck does he have to know anything about it? And consistency with serial links is not an argument. UDP will be very soon the default, and serial links for die-hard hobby use only. We should cater to that with our architecture and not stay stuck in the past.

dogmaphobic commented 9 years ago

QGroundControl opened for the longest time a default connection on UDP port 14550.

Ok. I re-read the very first line in this thread and NOW I understand what you're talking about. Not since I started working on QGC it ever started any connection automatically. You always had to click the Connect button. The same is true for all other GCs out there. That's why I kept not understanding it.

If QGC is to always start with an open/bound UDP link, what do we do with the Connect button? Does it apply to this (always present and connected) UDP link? This is a major departure from the current way things work. Note that I do agree 100% with you on this. I'm just wondering how the UI will work once UDP is always running and bound.

LorenzMeier commented 9 years ago

The question on the Connect button is a good one. If we would just rename it to "Add Connection" would that fix the semantic chaos? Because fundamentally that is what it's doing - its adding a link which is not there yet explicitely. Given your push for auto-config we implicitly already decided that other "links" are up and running by default.

dogmaphobic commented 9 years ago

Hmm... we have to think about this. The Connect button only says Connect when nothing is connected. Once there is any connection going, the button changes to Disconnect. You can go to the Link Management window and Add Connect an existing connection. When you do that, the Disconnect button turns into a drop down list, allowing you to choose which connection you want to disconnect.

For the sake of making it simple (for new users and in general), I would not have ANY button in the main tool bar. QGC is always bound to the UPD port and ready. Instead, there should be an easy way to get to a (better designed) Connection Management window. Using your email analogy, it would be the equivalent of the Accounts window, where you go to add/remove/enable/disable Accounts.

While at it, there should also be a simple way to Shut Down the UAV from the UI. If UDP is to become the standard, it implies there is a computer at the other end, which most likely needs to be shutdown cleanly before you remove power (yank the battery off of it).

DonLakeFlyer commented 9 years ago

@dogmaphobic This can be closed now?

dogmaphobic commented 9 years ago

Not really. @LorenzMeier suggested QGC should always start with an UDP link active (which I agree). That in turn changes the semantics of the Connect and most importantly, the Disconnect button. If QGC is indeed supposed to always start with the UDP link active, we need to rethink the whole Connect/Disconnect paradigm. In summary, don't change anything and close this or we need to further discuss the UX for it.

DonLakeFlyer commented 9 years ago

I'm not quite convinced on the no connect thing. The reason mail has no connect/disconnect is because it tends to be a more of a stateless connection. With a stateful connection you kind of need some sort being/end concept. Being state pulls parameters and waypoints. End state triggers flight log saving. What happens when you disconnect an automatically connected UDP connection. It just reconnects again right away? There is not some sort of end-flight thing instead? The semantics of automatic connection seem odd.

dogmaphobic commented 9 years ago

That's why we need to discuss further :) UDP is by nature, stateless. Things get sent and hopefully someone is at the other end to receive it. In the case of QGC, it would start with the UDP port open, ready to receive whenever someone sends something. The Start is basically the first packet received. Once we receive a packet, that end becomes "present" and from there, we rely on the heartbeat to know if it's still there. There is no End per se. Just a pause until we hear again (if ever). With that said, I cannot disagree with you either.

LorenzMeier commented 9 years ago

@DonLakeFlyer Since you're trying to build an application for users, the link management UI should follow the model of the user and should not try to follow any technical notion. That's a recipe for usability disaster.

The reason we connect serial ports is because the user is plugging a cable. There is no cable to plug for UDP, and consequently no affordance for any connection state. It has to be ready on boot and should not require any user intervention.

Do you own a Salae Logic analyser? If yes, upgrade to the newest Software they have and watch it operate. That's a device with a cable, and its a shining example of how to take the pain away of using it and not messing with connections even though there is clearly some USB connect state.

dogmaphobic commented 9 years ago

Do you own a Salae Logic analyser?

That made me laugh as I just got one a couple of months ago. Now ask me if I'm glad to get rid of sigrok :) Same analogy!

DonLakeFlyer commented 9 years ago

Let's work our way through this...

The concept of a conversation between QGC and a vehicle is stateful. The following states are required for QGC to run correctly:

In order to connect QGC to the mavlink stream it requires a connection to some piece of hardware to communicate through. For the hardware side of things, you can break it down into a couple steps:

So the question is how to trigger the QGC states and can they be automatic? Let's go through that...

Conversation Begin:

Conversation End:

I'm not against automatic everything at all, but I think it's way more complicated than it may seem to get correct. And the above questions need answers. I've provided my own opinions. I think it could work, but I'm concerned about the End->Idle->Begin automatic transition being quite odd. Also the mutliple connection case will get a bit odd. For me the question is whether the answers to these odditites will end up making things better or worse.

LorenzMeier commented 9 years ago

Hi,

First, I'd like to thank you for doing such an amazing job over the last months. It has allowed me to abstract from the technology and focus on the user experience, and its why we have this argument now. You need to trust me that I diligently do user acceptance tests with experts and non-experts and so what I present here are facts, not an interpretation of how things could be done.

What is really required is to let go of the concept that the hardware layer connection implies an application layer state. Its even just for telemetry a misfit: The fact that the radio modem is present has nothing to do with the drone being present. The same way saving the MAVLink log on closing the connection is not a natural fit, because the connection could be closed many minutes after landing or mid-flight. Saving and replaying MAVLink logs is a developer feature and never should be visible from a user-workflow perspective.

Really, consider an ISO/OSI model, consider MAVLink to be a broadcast packet on network interfaces and pretty please give up on point to point links. This is not how it has been architected and how it works.

It is also not true that one vehicle is connected "mainly" via one link. You just get MAVLink packets. You respond to the same vehicle via broadcast (and the hardware layer does some switching, the same way as a switch does switching instead of broadcasting like a hub). There is no "send to link" or point-to-point concept in MAVLink. The links we really care about are Wifi and LTE. Radio modems are in today's drone architectures as relevant as dial-up modems are today for computers. They are rapidly moving into a niche. Pixhawk 2 will come with Wifi. I personally care about radios because I will continue to use them, but they must not dictate the user workflow like they are doing now.

Even the USB connection to Pixhawk wouldn't technically require connection management: It could just auto-connect (and if there is more than one, ask to connect to which one). I'm not saying we should do that, but similarly to UDP there is no workflow need to expose this ever to the user.

You also don't need a connection state for the MAVLink stream at all. The "Save MAVLink log" dialog is extremely alienating to users (the #1 complaint in user acceptance testing). If you do away with it, then you don't need a state for the MAVLink stream either. Just keep logging until application close and allow users to re-find previous logs.

What you really need is a mission state: Pre-takeoff, flying, post-flight. That is what needs to drive the UI, because those are the dimensions the user is thinking in. It makes sense for users to save or upload a flight report when they have landed, it makes no sense to them to mess with MAVLink logs.

The same way as your mail client is not asking where to store the IMAP and SMTP transfer logs after its done sending and receiving emails.

DonLakeFlyer commented 9 years ago

You seem to be missing my point. We are saying the same things. You are providing an end-user perspective and I'm trying to figure out how to code it. I'm saying that all hardware level connection management can be handled automatically and mostly hidden from the user. That's only 90% true, but good enough to not have to discuss the 10% part.

I'll use your terminology to ask the most important questions, not getting into corner cases. The main issues come from how to automatically detect Post-flight and what happens after:

1) How do you trigger Post-flight state? Is it an automatic transition, if so triggered how? If automatic trigger, is it 100% reliable, if not what is the manual trigger? 2) If you are in Post-flight, how do you get back to Pre-flight or Flying. If automatic what is the trigger? 3) Should Post-flight state trigger a close of the mavlink log? I've been trying to make mavlink logs be per-flight. If mavlink logs don't need to be per-flight the answer is No. The fact that the current ui is annoying shouldn't be the reason to not want mavlink logs to be per flight. That is fixable.

If we can answer #1 and #2 and the answer to #3 is still no, then auto-magic everything is doable.

DonLakeFlyer commented 9 years ago

Some sort of mavlink message for takeoff and land that could be used to trigger on would work. Does that already exist? Also FYI: I'm fine with answer to #3 being No.

LorenzMeier commented 9 years ago

1) How do you trigger Post-flight state? Is it an automatic transition, if so triggered how? If automatic trigger, is it 100% reliable, if not what is the manual trigger?

Its all there. You just have to rely on the SYS_STATUS message and look at MAV_MODE. If mode == MAV_STATE_STANDBY you are in pre-flight, then it transitions to MAV_STATE_ACTIVE which is flying and finally goes back to MAV_STATE_STANDBY. Your UAS object will need to track the transitions to differentiate pre and post flight, but that's trivial. The whole safety state machine on the autopilot side depends on the landed detector, so the reliability question is irrelevant, because the GCS has to rely on the autopilot state. There is one catch: You might want to rely on arming instead because a mission which includes an intermediate landing operation will have a brief STANDBY sequence. So you really just might want to go for arming, which is in the same message. We will probably add auto-disarm, which will ensure that you get the full transition.

2) If you are in Post-flight, how do you get back to Pre-flight or Flying. If automatic what is the trigger?

Same answer as in 1).

3) Should Post-flight state trigger a close of the mavlink log? I've been trying to make mavlink logs be per-flight. If mavlink logs don't need to be per-flight the answer is No. The fact that the current ui is annoying shouldn't be the reason to not want mavlink logs to be per flight. That is fixable.

Since the onboard logging stops on disarming I would lock it to the disarmed state, not the flight state. This important as the operator "finishes" the session by disarming, while just landing doesn't mean the flight is over (he might have just touched down to deliver a package). The UI should not be modal though.

LorenzMeier commented 9 years ago

It would be great to get started on this and have the UDP link auto-connect as soon as possible. Even I keep stumbling over "why the heck is my RPI not talking to the GCS" because its entirely unexpected that I need to manually open a UDP port.

DonLakeFlyer commented 9 years ago

Since the onboard logging stops on disarming I would lock it to the disarmed state, not the flight state.

Turns out that there currently isn't a good way to make mavlink logs be per-flight and maintain full ui. The case that breaks is disarmm then re-arm and fly again without closing the connection. The reason being that in order to "replay" a log you have to capture the parameters coming across the wire. Without that, portions of the ui will not go live. Probably most important one being parameter editor, so you can check your settings.

Options: 1) For the disarm->re-arm without closing connection case: inject parameters into stream manually 2) Leave it as is and make QGC deal with no/partial parameter set on replay

I'd lean towards #1 since it provides better usability for replay. Otherwise it confusing as to why sometimes logs have parameters/ui and sometimes they don't.

dogmaphobic commented 8 years ago

This is long done with.