ADSB: Multi-vehicle compatibility and revamp #952
Conversation
There was a problem hiding this comment.
Light review only.
I think we generally try to avoid doing processing-heavy stuff in the mavlink callback; the idle callback may suffer latency but that's because it's more important to continue to process mavlink than process all of the content - thus the original structure.
MAVProxy/modules/mavproxy_adsb.py
Outdated
| def mavlink_packet(self, m): | ||
| '''handle an incoming mavlink packet''' | ||
| if m.get_type() == "ADSB_VEHICLE": | ||
| id = 'ADSB-' + str(m.ICAO_address) | ||
| label = str(m.callsign) + "/" + str(int(m.altitude * 0.001)) + "m" |
There was a problem hiding this comment.
This can be moved into where the we construct the ADSBVehicle
MAVProxy/modules/mavproxy_adsb.py
Outdated
| threat_radius_xy = get_threat_radius(self.threat_vehicles[id].state) if self.ADSB_settings.threat_radius_xy == 0 else self.ADSB_settings.threat_radius_xy | ||
| threat_radius_z = get_threat_radius(self.threat_vehicles[id].state) if self.ADSB_settings.threat_radius_z == 0 else self.ADSB_settings.threat_radius_z |
There was a problem hiding this comment.
It seems very odd looking inside threat_vehicles before we construct and add a vehicle if it doesn't exist.
Further, just because you can doesn't mean you should; these would be better written out as multiple lines.
MAVProxy/modules/mavproxy_adsb.py
Outdated
| threat_radius_xy = get_threat_radius(self.threat_vehicles[id].state) if self.ADSB_settings.threat_radius_xy == 0 else self.ADSB_settings.threat_radius_xy | ||
| threat_radius_z = get_threat_radius(self.threat_vehicles[id].state) if self.ADSB_settings.threat_radius_z == 0 else self.ADSB_settings.threat_radius_z |
There was a problem hiding this comment.
These settings need clarification; it looks like they're override values rather than default values. So name them appropriately if that's the case...
self.ADSB_settings.threat_radius_z_override
We should also not be using radius_z - it's not a radius :-)
MAVProxy/modules/mavproxy_adsb.py
Outdated
| if id not in self.threat_vehicles.keys(): # check to see if the vehicle is in the dict | ||
| # if not then add it | ||
| self.threat_vehicles[id] = ADSBVehicle(id=id, state=m.to_dict()) | ||
| self.threat_vehicles[id] = ADSBVehicle(id, m, threat_radius_xy, threat_radius_z) |
There was a problem hiding this comment.
This should probably end up creating the vehicle with a default radius, and then common code below this point updating it from the received message.
|
I've fixed it up - now using a similar approach to the AIS module. |
MAVProxy/modules/mavproxy_adsb.py
Outdated
| self.isThreat = False | ||
| self.onMap = False |
There was a problem hiding this comment.
Now using a mix of different variable-name styles; let's be consistent with the original format.
|
|
||
| self.add_command('adsb', self.cmd_ADSB, "adsb control", | ||
| ["<status>", "set (ADSBSETTING)"]) | ||
|
|
||
| self.ADSB_settings = mp_settings.MPSettings([("timeout", int, 5), # seconds | ||
| ("threat_radius", int, 200), # meters |
There was a problem hiding this comment.
This will break existing users. We should accept threat_radius as an argument and if it is set to something other than None then we should use its value in preference to the xy and z overrides
|
Testing went well, but I have (re)discovered that you really don't want multiple vehicles that are able to see each other's |
|
I've fixed up the variable naming and re-added threat_radius. If threat_radius is 0, then the xy/z overrides will be used instead. |
Modernisation of the ADSB Module: