mavgen_python: accept str for char and char[] message fields - #1238
mavgen_python: accept str for char and char[] message fields#1238MuhammadFathy wants to merge 1 commit into
Conversation
Passing a str for a char/char[] field raised
TypeError: must be str or None, not bytes
from the generated message constructor, e.g.
mav.statustext_send(mavutil.mavlink.MAV_SEVERITY_NOTICE, "hello")
as MAVProxy's example module does. Commit 667cfcd started splitting the
raw value on b"\x00" in __init__, which only works for bytes-like input,
so every caller that had been passing a str broke.
Add a char_field_to_bytes() helper to the generated module and route
char field assignment through it, so str, bytes and bytearray are all
accepted and normalised to bytes before the NUL split and before being
handed to struct.pack. str is encoded as ascii with errors="replace",
matching the decode already performed on the same line.
The constructor and *_encode/*_send annotations for char fields are
widened from bytes to the new CharFieldLike alias so the generated
dialects still type-check.
The second case mentioned in the issue, mavftp.py's
op.payload.split(b"\x00"), is not affected: op.payload is a bytearray
and bytearray.split() accepts a bytes separator.
Adds tests/test_mavgen_python.py, which generates the common dialect and
exercises str/bytes/bytearray input, NUL truncation, non-ascii handling
and a full pack/parse round trip.
Fixes ArduPilot#1225
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fallenmi
left a comment
There was a problem hiding this comment.
The intended input paths work: on exact head 3154af13, str, bytes, and bytearray produce identical wire bytes for both char and char[], and the five added tests pass. I also applied the head over current master 449d03c2; the same focused tests pass there.
There is one blocking regression in char_field_to_bytes(): after the bytes and str branches, return bytes(value) does not only handle the declared bytearray case. It also accepts integers, integer iterables, memoryview, and even dictionaries with integer keys. The base constructor rejects all of these immediately, but this head makes MAVLink_statustext_message(6, 100) produce 100 NUL bytes and successfully pack as an empty STATUSTEXT; {65: "x"} packs as text A. That turns common caller type errors into valid but unintended telemetry, and a large integer can allocate that many bytes before the fixed-width pack truncates it.
Please handle bytearray explicitly and raise TypeError for values outside CharFieldLike, with a regression covering at least an integer or integer iterable. The exact head currently has no GitHub check contexts or Actions run, so upstream CI also still needs to execute after the update.
Reviewed with OpenAI Codex assistance; I generated dialects from the exact PR base, exact head, and current-master merge and reproduced the constructor and wire-packet behavior locally.
Passing a str for a char/char[] field raised
from the generated message constructor, e.g.
as MAVProxy's example module does. Commit 667cfcd started splitting the raw value on b"\x00" in init, which only works for bytes-like input, so every caller that had been passing a str broke.
Add a char_field_to_bytes() helper to the generated module and route char field assignment through it, so str, bytes and bytearray are all accepted and normalised to bytes before the NUL split and before being handed to struct.pack. str is encoded as ascii with errors="replace", matching the decode already performed on the same line.
The constructor and _encode/_send annotations for char fields are widened from bytes to the new CharFieldLike alias so the generated dialects still type-check.
The second case mentioned in the issue, mavftp.py's op.payload.split(b"\x00"), is not affected: op.payload is a bytearray and bytearray.split() accepts a bytes separator.
Adds tests/test_mavgen_python.py, which generates the common dialect and exercises str/bytes/bytearray input, NUL truncation, non-ascii handling and a full pack/parse round trip.
Fixes #1225