Documentation doesn't clearly state what happens after an abort

[atomictom]

By @hamishwillee's suggestion, I'm opening this because I was trying to figure out what happens after a connection abort (i.e. heartbeat_timeout is exceeded), and the documentation does not make it clear. After looking through dronekit/init.py, I see on lines 1099-1102:

                if self._heartbeat_error and self._heartbeat_error > 0 and time.time(
                ) - self._heartbeat_lastreceived > self._heartbeat_error:
                    raise APIException('No heartbeat in %s seconds, aborting.' %
                                       self._heartbeat_error)

Looking at dronekit/mavlink.py, I can see that the exception will cause the handler thread to exit:

            try:
                while True:
                    # Downtime
                    time.sleep(0.05)

                    # Loop listeners.
                    for fn in self.loop_listeners:
                        fn(self)

...

            except APIException as e:
                errprinter('>>> ' + str(e.message))
                self._alive = False
                self.master.close()
                self._death_error = e
                return

I think the documentation could be improved by stating that the abort will cause the connection to permanently close (while a heartbeat_warning will not), and that if this behavior is not desired, to set the timeout to a large value. If an abort occurs, @hamishwillee suggested that the only recourse would be a reconnect, and the documentation should state this. Also, in the case of an abort, I'm not sure how to detect it from a script (to go about doing a reconnect).

One other issue, is that a reconnect would issue a new vehicle object, but all the decorated message/attribute listeners on the top level (say in a module) would have already been initialized, and would not be updated. Meaning if you expect a reconnect, you would need to re-declare all your decorated handlers with the new vehicle object (probably inside an initialization function).

Edit to illustrate my point:

# Is only called when the module is imported, cannot be updated...
@vehicle.on_attribute('location')
def listener(...)
    ...

def initialize(vehicle):
    # Called each time we call initialize. Sets up listeners on new vehicle
    @vehicle.on_attribute('location') 
    def listener(...)
         ...

It might also be worth considering if it should be possible to disable it completely (say, pass in a timeout of -1).

[mikerob]

I'll post an example of how I'm handling a timeout. It is problematic having the connection drop completely - MAVProxy will continue to listed in the relevant port until something comes back.

Being able to separate initial connection timeout from mid-connection timeout would be useful. In my use case, I'd like to disable the mid-connection timeout completely but still have a short initial one for detecting connections.

*** EDIT: https://github.com/mikerob/dronekit-python/blob/master/pigs/mavmaster.py

[atomictom]

@mikerob That's a good point. I would also like to be able to have the initial connect call timeout more quickly, but then either not have connection abort or have the abort occur after a much longer period of time. (It might be good to still have it abort after some time so you can try connecting in an alternate way.)

[hamishwillee]

@tcr3dr Some feedback on our connection API here. The requests (in summary) are:

• Separately configurable initial and while active connection timeout. I think this is reasonable if only because I don't like waiting 30s to fail the initial connection, but I might way 30 seconds to fail when things are going well.
• Ability to set "never time out". This is reasonable enough now that users can implement their own timeout handling
• WE ALSO MISSED ability to check the vehicle for "valid state" - ie after abort or closing the vehicle it is no longer valid - IMO this should be checkable, perhaps by making any call the the object raise an appropriate exception.

There are other points in there about continuing to listen for the port until something comes back - though obviously it isn't MAVProxy :-)

I'm not sure about the point about decorators/initialisation at module level requiring re-initialisation. Yes it is true, but having the decorators at that level sounds like a design problem with the app, not our architecture.

There is an action on me here to document what happens after abort - but I'd like your feedback on the above before I write it.

[atomictom]

@hamishwillee

I'm not sure about the point about decorators/initialisation at module level requiring re-initialisation. Yes it is true, but having the decorators at that level sounds like a design problem with the app, not our architecture.

Mostly that there should probably be an explicit mention of this in the documentation. I actually use Flask for my web server and the idiom there is to have url routes as decorators on module level functions like so:

@app.route('/')
def index():
    ...

I imagine others may think that is the natural approach, for example, they may have a package where the init.py creates the vehicle object, then imports the attribute/message listeners which are top-level functions in a sub-module. Perhaps:

drone/init.py

vehicle = connect(...)

import script

drone/script.py

from drone import vehicle

@vehicle.on_attribute('location')
def listener(...)
    ...

However, if aborts require calling connect() again, then that wouldn't work. Obviously, this is an architecture problem that would need to be solved by the user, but it strikes me as a problem that could come up a lot.

[mikerob]

Is it possible to create a "vehicle.reconnect("/dev/tty...etc")" function that you would use to revive a connection, while keeping everything else intact?

[atomictom]

@mikerob While I like the idea of being able to keep the connection from timing out at all, I like this idea as well. The nice thing about what you're suggesting is you can try to connect on another channel than whatever the first connection used.

[Evidlo]

Has anything changed here? I haven't found a decent way to detect an abort so that I can reconnect to the Pixhawk if it loses power. Ideally there would be a vehicle.aborted attribute that I could attach a listener to.

Here's one failed attempt at a solution:

from dronekit import connect
from time import sleep

def last_heartbeat_callback(vehicle, attr_name, last_heartbeat):
    if last_heartbeat > 5:
        print('last_heartbeat > 5, reconnecting')
        vehicle.close()
        pixhawk_connect()

def pixhawk_connect():
    vehicle = connect("tcp:localhost:14550", heartbeat_timeout=50)
    vehicle.add_attribute_listener('last_heartbeat', last_heartbeat_callback)

pixhawk_connect()

while True:
    sleep(1)

But I get

>>> Exception in attribute handler for last_heartbeat
>>> cannot join current thread

on vehicle.close(), probably because I'm trying to close vehicle from inside its own callback.

[Evidlo]

Here's a working solution, but too much of a hack in my opinion:

from dronekit import connect
from time import sleep

def last_heartbeat_callback(vehicle, attr_name, last_heartbeat):
    if last_heartbeat > 5:
        print('last_heartbeat > 5, reconnecting')
        vehicle._handler.master.close()
        pixhawk_connect()

def pixhawk_connect():
    vehicle = connect("tcp:localhost:14550", heartbeat_timeout=50)
    vehicle.add_attribute_listener('last_heartbeat', last_heartbeat_callback)

pixhawk_connect()

while True:
    sleep(1)