Expose heartbeat parameter(s) or some other method of detecting active connection

[mikerob]

Under DKPY1, this was raised: #112

DKPY2 already appears to monitor heartbeat status, but there's no way to directly acess it, or (it seems) reliably detect an active connection.

For example, I fI run vehicle.close(), I can still access all of the saved data and there's no clear indication I've lost my connection.

Just being able to see either time of last heartbeat or time since last heartbeat would allow the user to manage timeouts themselves with staged behaviour.

I see there's an exception thrown, but that's after 30s (which isn't customisable). IF you want to catch a timeout before that, you don't appear to have an option.

[hamishwillee]

@tcr3dr Interested in how you plan to fix this.
IMO it would definitely be a good idea to have the timeout customisable.

It makes me cringe a bit, but I'm sure some people wouldn't mind having the value of the timeout timer exposed as an attribute either - perhaps slightly nicer to have as a string "Connected", "Cardiac arrest", "Dead, Jim" (though time for each of those might be a bit painful to tune because it will depend on the connection.

We do need some way to know that after vehicle.close() the vehicle is dead. Can the object set itself to None as a final act of suicide?

[tcr3dr]

@mikerob You can change the initial heartbeat_timeout in the connect() method, but not after the fact.

I suppose we need five properties:

1. vehicle.connected — read-only bool if we are a live or dead connection (dead following timeout or .close()`
2. vehicle.link_established — read-only bool if we have crossed the heartbeat_threshold. this is independent of .connected and meant as a warning under congested situations
3. vehicle.heartbeat_timeout — read-write integer of how long since last heartbeat we close connection (normally 30s)
4. vehicle.heartbeat_threshold — read-write integer of how long since last heartbeat we determine link lost (normally 5s)
5. vehicle.last_heartbeat — read-only time since last heartbeat

Happy to bikeshed any of these. These should be easy to get into DKPY2, but let's settle on a minimal, "good enough" solution so we can land these soon.

[tcr3dr]

@hamishwillee Objects cannot set themselves to None, for which the immutable functional purist in me is grateful, and the OO demon rages bitterly.

[mikerob]

"Connected" and "last_heartbeat" would be a minimal, functional set of parameters. Connected works for "basic" users, and advancred users can derive everything else from last_heartbeat.

My $.02

Sent from my mobile device

On 14 Nov 2015, at 8:24 am, Tim Ryan notifications@github.com wrote:

@mikerob You can change the initial heartbeat_timeout in the connect() method, but not after the fact.

I suppose we need five properties:

vehicle.connected — read-only bool if we are a live or dead connection (dead following timeout or .close()`
vehicle.link_established — read-only bool if we have crossed the heartbeat_threshold. this is independent of .connected and meant as a warning under congested situations
vehicle.heartbeat_timeout — read-write integer of how long since last heartbeat we close connection (normally 30s)
vehicle.heartbeat_threshold — read-write integer of how long since last heartbeat we determine link lost (normally 5s)
vehicle.last_heartbeat — read-only time since last heartbeat
Happy to bikeshed any of these. These should be easy to get into DKPY2, but let's settle on a minimal, "good enough" solution so we can land these soon.

—
Reply to this email directly or view it on GitHub.

[hamishwillee]

IMO the only connection API we should supply is vehicle.last_heartbeat. This should have a value of None before a connection is established and we're getting heartbeats. After that it should be time since we got the last heartbeat (or possibly time since we got "message of any kind" from the vehicle).

My reasoning is that any other information implies that we really have a connection (ie are receiving and sending messages). In fact the only thing we KNOW is the freshness of the last message. I want this in people's minds. I don't see adding a boolean they can check on as making this any simpler than just defining a callback on the time value.

We will still need something to perform the function of vehicle.connected but it really needs to imply "valid vehicle" rather than connection state (ie solely for vehicle.close() and as a backup if the user does not connection monitoring.. I would also propose that we use it to (after removing any callbacks) cause other function calls on the object to raise an exception . That will encourage the developer to immediately set the value to None themselves (a familiar pattern for those who learned C++ first :-0 ). If we do it this way probably no need to have an API to set this timeout time.

Irrespective of whether we have the additional boolean for tracking a timeout we still need to think about what advice we give people for how they take action on it/set the value.

Any ideas?

My thoughts ...

The vehicle.last_heartbeat value is a measure of the "freshness" of information from the vehicle. Scripts which rely on timely position information may wish to modify their behaviour based on this value (for example when tracking a moving target).

Higher values of this attribute (>5 seconds, say) may indicate that the link is broken (in which case developers may wish to pause or restart the script).

Note: The connection to the vehicle is potentially "lossy": Irrespective of the last_heartbeat value you cannot assume that messages are (or are not) getting to the vehicle.

... then having an example that perhaps just exits the script following a 6 second timeout?

[mikerob]

I implemented heartbeat_lastreceived on my branch over the weekend for my GCS.

I'd agree that that's the only thing you NEED to expose.

Turns out heartbeat_timeout is triggered at 5s, not at the same time as the "connection". Given how in flux the timeout interface is, exposing just the time since the last one is the clearest, simplest and most user flexible option.

On 16 Nov 2015, at 12:46 pm, Hamish Willee notifications@github.com wrote:

IMO the only connection API we should supply is vehicle.last_heartbeat. This should have a value of None before a connection is established and we're getting heartbeats. After that it should be time since we got the last heartbeat (or possibly time since we got "message of any kind" from the vehicle).

My reasoning is that any other information implies that we really have a connection (ie are receiving and sending messages). In fact the only thing we KNOW is the freshness of the last message. I want this in people's minds. I don't see adding a boolean they can check on as making this any simpler than just defining a callback on the time value.

We will still need something to perform the function of vehicle.connected but it really needs to imply "valid vehicle" rather than connection state (ie solely for vehicle.close() and as a backup if the user does not connection monitoring.. I would also propose that we use it to (after removing any callbacks) cause other function calls on the object to raise an exception . That will encourage the developer to immediately set the value to None themselves (a familiar pattern for those who learned C++ first :-0 ). If we do it this way probably no need to have an API to set this timeout time.

Irrespective of whether we have the additional boolean for tracking a timeout we still need to think about what advice we give people for how they take action on it/set the value.

Any ideas?

My thoughts ...

The vehicle.last_heartbeat value is a measure of the "freshness" of information from the vehicle. Scripts which rely on timely position information may wish to modify their behaviour based on this value (for example when tracking a moving target).

Higher values of this attribute (>5 seconds, say) may indicate that the link is broken (in which case developers may wish to pause or restart the script).

Note: The connection to the vehicle is potentially "lossy": Irrespective of the last_heartbeat value you cannot assume that messages are (or are not) getting to the vehicle.

... then having an example that perhaps just exits the script following a 6 second timeout?

—
Reply to this email directly or view it on GitHub.

[tcr3dr]

See #457.

[tcr3dr]

#457 is merged. Let's see what ideas come out of this use and what we want to integrate as first-class features in the future.

[hamishwillee]

Too right. I think what you've done is great - flexible enough for people to easily implement any other form of monitoring they like.