driver: Allow user to specify the source system, rather than assume a default value - #94
driver: Allow user to specify the source system, rather than assume a default value#94joshanne wants to merge 1 commit into
Conversation
| self.data = data | ||
|
|
||
| def io_process(url, bus, target_system, baudrate, tx_queue, rx_queue, exit_queue, parent_pid): | ||
| def io_process(url, bus, source_system, target_system, baudrate, tx_queue, rx_queue, exit_queue, parent_pid): |
There was a problem hiding this comment.
Perhaps this is more correct to append the source system at the end of the argument list for compatibility?
57da112 to
20d0667
Compare
|
@tridge Can I bump this one for review? GUI Tool requires this change to be able to set the |
fallenmi
left a comment
fallenmi
left a comment
There was a problem hiding this comment.
The configurable source-system path is wired correctly, but this introduces a backwards-compatibility regression in MAVCAN.is_mavlink_port(). The existing two-argument call (device_name, baudrate) now raises TypeError before opening the connection because source_system became required.
The facade and constructor already preserve the prior value when the new option is omitted. Please retain the same compatibility here:
def is_mavlink_port(device_name, baudrate, source_system=250):I verified that the three-argument path still forwards the requested value to mavutil.mavlink_connection, so the default preserves old callers without changing the new behavior.
Disclosure: I used Codex to inspect this exact revision and run the focused compatibility oracle; I verified the result.
… default value This one has bitten me a few times on systems that have SYSID_MY_GCS (or the new MAV_GCS_SYSID and MAV_GCS_SYSID_HI) parameters configured with enforcement of the ground station in control (ie. SYSID_ENFORCE or MAV_OPTIONS bit#0). This is the pydronecan changes that allow the front-end to configure the source_system as required
20d0667 to
f023caf
Compare
fallenmi
left a comment
fallenmi
left a comment
There was a problem hiding this comment.
The previous compatibility blocker is resolved at exact head f023cafc0baa4f56bb9e31ba0c8e60ebac627166: MAVCAN.is_mavlink_port() again defaults source_system to 250, while explicit three-argument callers still pass through their requested system ID. The old-to-new delta changes only that default; the already-verified facade blob is unchanged.
I checked both exact bindings: the two-argument call resolves to 250 and an explicit third argument resolves to the supplied value (2/2 passed). GitHub's current merge tree is identical to the head tree, and the refreshed Python 3.8–3.14 Actions matrix is green (7/7 jobs, including lint, unittest, and pytest). The only annotations are repository-level Node.js deprecation warnings for the Actions versions.
Disclosure: I used OpenAI Codex to inspect this exact revision and run the focused compatibility oracle; I verified the refs, result, interactions, policy, and CI before submission.
2 participants
This one has bitten me a few times on systems that have SYSID_MY_GCS (or the new MAV_GCS_SYSID and MAV_GCS_SYSID_HI) parameters configured with enforcement of the ground station in control (ie. SYSID_ENFORCE or MAV_OPTIONS bit#0).
This is the
pydronecanchanges that allow the front-end to configure the source_system as required.Required by: dronecan/gui_tool#108