Support filtering incoming messages based on target_id

[peterbarker]

No description provided.

[hamishwillee]

1. Tests need to pass :-)

2. I'm not sure filtering on None quite makes sense here - when would it ever be useful for a vehicle not to filter anything?

3. If target system is 0 we do no filtering at all. Is that intentional?

4. I'm a bit concerned this doesn't do quite what we think (though it may "do the right job")

   if self._target_system:
     src = msg.get_srcSystem()
     if src != self._target_system and src != 0:
        return
   

   Specifically what we're getting back is msg.get_srcSystem(), which is appears to be always the source id of the message. We're not filtering on "messages that were sent to us or broadcast" but on "messages that have the id we think is our target vehicle".

The assumption in this code is that 0 is the sourceSystem for a broadcast message, and I don't think it is. I think that we probably don't see messages unless they are either intended for us or for broadcast. We need to confirm this.

Also if 0 was a broadcast message then this still wouldn't help us because that would mean we would be getting any broadcast heartbeats (and these might be setting our mode).

Anyway, if I'm right, then we don't need the filter on src != 0. However we do need to consider whether we will process broadcast messages at all. Is there a case where we ever want to read messages from anything that isn't the vehicle?

Last of all, we need to think about working out what our target id should be. A default of 1 should work in most cases but we probably should think about code to detect the value if at all possible. Minimally something to help people if connect fails to understand this is a likely cause.

[mikerob]

Remember there are two use cases for DroneKit - at the vehicle end AND as a GCS.

The GCS case is where you would want to process ALL vehicle IDs.

On 24 Feb 2016, at 4:21 pm, Hamish Willee notifications@github.com wrote:

Tests need to pass :-)
I'm not sure filtering on None quite makes sense here - when would it ever be useful for a vehicle not to filter anything?
If target system is 0 we do no filtering at all. Is that intentional?
I'm a bit concerned this doesn't do quite what we think (though it may "do the right job") if self._target_system: src = msg.get_srcSystem() if src != self._target_system and src != 0: return Specifically what we're getting back is msg.get_srcSystem(), which is appears to be always the source id of the message. We're not filtering on "messages that were sent to us or broadcast" but on "messages that have the id we think is our target vehicle".
The assumption in this code is that 0 is the sourceSystem for a broadcast message, and I don't think it is. I think that we probably don't see messages unless they are either intended for us or for broadcast. We need to confirm this.

Anyway, if I'm right, then we don't need the filter on src != 0. However we do need to consider whether we will process broadcast messages at all. Is there a case where we ever want to read messages from anything that isn't the vehicle?

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

[hamishwillee]

@mikerob Well you'd want to process all ids, but I think that given the 1:1 nature of the connection between a vehicle and an autopilot you would do that through separate vehicle objects - upshot that changing to only intercept the one specified id makes a lot of sense.

What we would need to change to properly support this model though is a better way to discover all the vehicles on the network and return the Vehicle object that makes sense for each. I'd see that as "step2". Perhaps in the mean time have vehicle get only messages from the supported vehicle, and any vehicle if 0 is set as the target.

[mrpollo]

should we deal with this at the forward_message level?

[peterbarker]

On Tue, 12 Apr 2016, Ramon Roche wrote: should we deal with this at the forward_message level?

Further patches in this PR move this code into mavlink.py's MAVSystem object - so, yes, sort of :-)

[mrpollo]

this patch mostly makes sense to me, but it uncovers some new scenarios we might have not been ready to address here.

What is DroneKit doing now:

• Vehicle state
• Modifying state
• Reacting to mavlink and vehicle attribute state changes

What is our core backend good for right now (mavlink):

• vehicle messages
• GCS messages

In theory we should always assume we are not the only link in the network connected to a Flight Controller, this PR makes sense to me because it allows us to filter through vehicle messages, but it doesn't do much with all the GCS (or broadcast) messages, I think we might need to add a new state machine for all the broadcasted messages, this might have a big cost since most users won't really care so we need to be careful how we add this new state machine.

@peterbarker just make sure our tests pass and I'll proceed with merging, if you need help I'll gladly commit some time to get this merged this is big.

[mrpollo]

@peterbarker are you still working on this?

[peterbarker]

On Mon, 29 Aug 2016, Ramon Roche wrote: @peterbarker are you still working on this?

Yes.

[amilcarlucas]

This looks good Peter. I would like to try it. Can you please rebase ?

[peterbarker]

@amilcarlucas Rebased.

[mrpollo]

Good to see you reviving old branches @peterbarker 👍

[peterbarker]

On Tue, 17 Oct 2017, Ramon Roche wrote: Good to see you reviving old branches @peterbarker 👍

:-) This and the pymavlink one are some of my biggest disappointments, actually. They add some really nice functionality, but I haven't managed to get the pymavlink changes in (yet?). This one's been pretty stable... a few people have used it relatively recently IIUC....

[guglie]

Thank you @peterbarker !

[amilcarlucas]

Thank you Peter. Any clues why the tests fail ?

[peterbarker]

I'm closing this:

• it doesn't rebase cleanly
• pymavlink has support for multiple vehicles in a different way now, so the required patches for pymavlink won't be going in

Somebody'll have to work out how we do dronekit-python with multiple targets in pymavlink using the new pymavlink options :-)