driver: Allow pydronecan to set AutoPilot Parameter - #93
Conversation
Allows the driver to send a param set request to the autopilot, meaning we can programatically set parameters from dronecan_gui_tool if necessary
25e824b to
83c150e
Compare
|
@joshanne can you explain the use case? |
|
Certainly - with my dronecan gui tool plugin PR, I was making use of this to set parameters in the autopilot. I could see something like this being used to set the CAN_SLCAN_CPORT without the need for using a dedicated GCS to make the changes. I could also see users making their own plugins for the CAN tool to make parameter changes through to the AutoPilot. I'm not necessarily saying this is the right way to do this, but exposing the parameter interface of pymavlink through the pydronecan means I'm able to use the existing connection to send on the request. |
fallenmi
left a comment
There was a problem hiding this comment.
SetParam drops the MAVCAN driver's selected target. The driver carries mavlink_target_system as its local target_system and uses it explicitly for CAN-forward and CAN-frame packets, but the new handler calls conn.param_set_send(name, value) without passing or synchronizing that selection. Pymavlink's wrapper addresses PARAM_SET through the connection object's separate target, which starts at zero and then locks onto the first valid vehicle heartbeat. The result can be a broadcast parameter write or a write to a different vehicle from the one selected for MAVCAN.
I reproduced this at the wire level with mavlink_target_system=42. In both cases the existing MAV_CMD_CAN_FORWARD packet correctly targeted system 42. The new public set_parameter() call emitted PARAM_SET.target_system=0 before any heartbeat, then target_system=7 after an unrelated valid heartbeat from system 7. Exact head 83c150eb5442225a79fd8e9530a73770beeb4119 and the exact current-master synthetic merge reproduced that pair 10/10 each. A two-line diagnostic control that synchronized pymavlink's target from the driver's existing local target emitted (42, 42) 10/10 on both head and current-master integration.
Please make the parameter write use the same selected/learned target as the rest of MAVCAN, and add wire-level coverage for both the pre-heartbeat case and an unrelated first vehicle heartbeat. This is important for multi-vehicle links because the current implementation can silently modify the wrong autopilot.
All 58 repository tests pass on exact base, head, and current-master integration, and the critical flake8 gate is clean. The seven green upstream jobs do not install pymavlink, so they cannot import or exercise this optional MAVCAN path.
AI disclosure: I used OpenAI Codex to inspect the exact diff, construct and run the MAVLink wire oracle and diagnostic control, and draft this review. I verified the packets, commit IDs, source paths, outputs, and conclusion.
These changes allow pydronecan to call into pymavlink to set a parameter on the AutoPilot.