From 61c56d9c426e28f859c160fcd3138b3aafd609a3 Mon Sep 17 00:00:00 2001 From: Hamish Willee Date: Fri, 5 Jun 2026 12:07:48 +1000 Subject: [PATCH 01/14] mavgen_c: Helper functions to get CMD metadata and range check params --- generator/mavgen_c_cmd_helpers.py | 346 ++++++++++++++++++++++++++++++ generator/mavparse.py | 13 +- 2 files changed, 357 insertions(+), 2 deletions(-) create mode 100644 generator/mavgen_c_cmd_helpers.py diff --git a/generator/mavgen_c_cmd_helpers.py b/generator/mavgen_c_cmd_helpers.py new file mode 100644 index 000000000..8f1a452ec --- /dev/null +++ b/generator/mavgen_c_cmd_helpers.py @@ -0,0 +1,346 @@ +#!/usr/bin/env python3 +"""Generate mavlink_cmd_helpers.h — per-MAV_CMD range-check tables and helpers. + +Outputs a header with: + - param_bounds[] explicit min/max from XML minValue/maxValue attributes + - cmd_flags_table[] hasLocation and isDestination flags from XML + - Helper functions: is_sentinel, lat_in_range, lon_in_range, + cmd_flags, has_location, is_destination, check_range + +Only commands in SUPPORTED_CMDS (matching SupportedCommandParams in +mavlink_command_params.h) are included in the tables. +""" + +import argparse +import math +import os +import sys + +# Allow package-relative imports when invoked as a script. +# The grandparent of this file is the directory that contains the +# "pymavlink" package (e.g. the repo root or site-packages parent). +if __name__ == "__main__" and __package__ is None: + _here = os.path.dirname(os.path.abspath(__file__)) # generator/ + _pymavlink = os.path.dirname(_here) # pymavlink/ + _root = os.path.dirname(_pymavlink) # / + if _root not in sys.path: + sys.path.insert(0, _root) + +from pymavlink.generator import mavparse + +# Must match SupportedCommandParams[] in src/modules/mavlink/mavlink_command_params.h +SUPPORTED_CMDS = { + 16, 17, 19, 20, 21, 22, 31, 80, 84, 85, 93, 112, 114, 176, 177, 178, 179, 189, + 195, 196, 197, 201, 206, 211, 212, 214, 400, 420, 530, 532, 534, 2000, 2001, + 2003, 2500, 2501, 3000, 4501, 5000, 5001, 5002, 5003, 5004, 5100, 42600, +} + + +def load_with_includes(xml_path): + """Parse xml_path and all transitively included files, merge enums into root.""" + base = os.path.dirname(os.path.abspath(xml_path)) + root = mavparse.MAVXML(xml_path, wire_protocol_version=mavparse.PROTOCOL_2_0) + all_xml = [root] + seen = {os.path.abspath(xml_path)} + queue = [os.path.join(base, f) for f in root.include] + while queue: + path = os.path.abspath(queue.pop(0)) + if path in seen: + continue + seen.add(path) + try: + x = mavparse.MAVXML(path, wire_protocol_version=mavparse.PROTOCOL_2_0) + all_xml.append(x) + queue += [os.path.join(os.path.dirname(path), f) for f in x.include] + except Exception: + pass + mavparse.merge_enums(all_xml) + return root + + +def float_c(v): + """Format a Python float as a C float literal, using NAN for nan values.""" + if math.isnan(v): + return "(float)NAN" + s = repr(float(v)) + # Ensure there's always a decimal point for clarity + if "." not in s and "e" not in s: + s += ".0" + return s + "f" + + +def generate(xml_path, output_path): + root = load_with_includes(xml_path) + + mav_cmd = next((e for e in root.enum if e.name == "MAV_CMD"), None) + if mav_cmd is None: + raise RuntimeError("MAV_CMD enum not found in %s" % xml_path) + + # Collect entries for supported commands, sorted by cmd value + entries = sorted( + (e for e in mav_cmd.entry if int(e.value) in SUPPORTED_CMDS), + key=lambda e: int(e.value), + ) + + # Build param bounds table — only params with at least one bound defined + bounds = [] # list of (cmd, param_1based, lo_float, hi_float) + for entry in entries: + cmd = int(entry.value) + for param in entry.param: + if param.reserved: + continue + lo_s = (param.minValue or "").strip() + hi_s = (param.maxValue or "").strip() + if lo_s or hi_s: + lo = float(lo_s) if lo_s else float("nan") + hi = float(hi_s) if hi_s else float("nan") + bounds.append((cmd, int(param.index), lo, hi)) + + # Build cmd flags table + flags = [] # list of (cmd, flags_byte, short_name) + for entry in entries: + cmd = int(entry.value) + f = 0 + if getattr(entry, "has_location", False): + f |= 1 # CMD_FLAG_HAS_LOCATION + if getattr(entry, "is_destination", False): + f |= 2 # CMD_FLAG_IS_DESTINATION + short = entry.name.replace("MAV_CMD_", "") + flags.append((cmd, f, short)) + + with open(output_path, "w") as out: + _write(out, bounds, flags) + + +_HEADER_TOP = """\ +/* AUTO-GENERATED by mavgen_cmd_helpers.py from {xml} — do not edit. + * Regenerated by the build system when the mavlink submodule or XML changes. + */ +#pragma once + +#include +#include + +#ifdef __cplusplus +namespace mavlink_cmd_helpers {{ +#endif + +/* ------------------------------------------------------------------- + * Sentinel detection + * + * is_sentinel(): non-zero if v means "param not provided". + * NaN is the MAVLink standard sentinel. + * ±0.0 is also accepted by default: many GCS tools send 0 for unused + * fields rather than NaN. + * + * Define MAVLINK_CMD_STRICT_SENTINEL (e.g. -DMAVLINK_CMD_STRICT_SENTINEL + * in a test build) to reject ±0.0 and require NaN only — useful for + * auditing senders that send 0 instead of NaN. + * ------------------------------------------------------------------- */ +static inline int is_sentinel(float v) +{{ +\tuint32_t bits; +\t__builtin_memcpy(&bits, &v, sizeof(bits)); +\tif ((bits & 0x7F800000u) == 0x7F800000u) return 1; /* NaN — always a sentinel */ +#ifndef MAVLINK_CMD_STRICT_SENTINEL +\tif (!(bits & 0x7FFFFFFFu)) return 1; /* ±0.0 — default sentinel */ +#endif +\treturn 0; +}} + +/* is_coord_sentinel(): additionally treats INT32_MAX-derived values as + * sentinel. MISSION_ITEM_INT / COMMAND_INT use INT32_MAX (≈ 2.15e9 + * as float) in params 5/6 to mean "use current position". */ +static inline int is_coord_sentinel(float v, int is_int_frame) +{{ +\treturn is_sentinel(v) || (is_int_frame && (v >= 2.0e9f || v <= -2.0e9f)); +}} + +/* ------------------------------------------------------------------- + * Implicit geographic range helpers + * + * is_int_frame=1: raw int32 (MISSION_ITEM_INT, or COMMAND_INT global + * before /1e7 conversion in handle_message_command_int). + * lat ∈ [−9×10⁸, 9×10⁸] (±90° × 1e7) + * lon ∈ [−1.8×10⁹, 1.8×10⁹] (±180° × 1e7) + * is_int_frame=0: float degrees (COMMAND_LONG, or COMMAND_INT after + * the receiver has applied the 1e7 scale). + * lat ∈ [−90, 90], lon ∈ [−180, 180] + * ------------------------------------------------------------------- */ +static inline int lat_in_range(float v, int is_int_frame) +{{ +\tif (is_coord_sentinel(v, is_int_frame)) return 1; +\treturn is_int_frame ? (v >= -9e8f && v <= 9e8f) +\t : (v >= -90.0f && v <= 90.0f); +}} + +static inline int lon_in_range(float v, int is_int_frame) +{{ +\tif (is_coord_sentinel(v, is_int_frame)) return 1; +\treturn is_int_frame ? (v >= -1.8e9f && v <= 1.8e9f) +\t : (v >= -180.0f && v <= 180.0f); +}} + +""" + +_TABLES_MID = """\ + +/* Per-command attribute flags from XML hasLocation / isDestination. */ +struct CmdFlags {{ uint16_t cmd; uint8_t flags; }}; + +#define CMD_FLAG_HAS_LOCATION (1u << 0u) +#define CMD_FLAG_IS_DESTINATION (1u << 1u) + +""" + +_HEADER_BOTTOM = """\ + +/* ------------------------------------------------------------------- + * Public helpers + * ------------------------------------------------------------------- */ + +/* cmd_flags(): binary search for command attribute flags. + * Returns the flags byte, or -1 if cmd is not in the table. */ +static inline int cmd_flags(uint16_t cmd) +{{ +\tunsigned lo = 0u, hi = cmd_flags_count; +\twhile (lo < hi) {{ +\t\tunsigned mid = lo + (hi - lo) / 2u; +\t\tif (cmd_flags_table[mid].cmd == cmd) return (int)cmd_flags_table[mid].flags; +\t\telse if (cmd_flags_table[mid].cmd < cmd) lo = mid + 1u; +\t\telse hi = mid; +\t}} +\treturn -1; +}} + +/* has_location(): 1 if cmd carries lat/lon/alt, 0 if not, -1 if unknown. */ +static inline int has_location(uint16_t cmd) +{{ +\tconst int f = cmd_flags(cmd); +\treturn f < 0 ? -1 : ((f & (int)CMD_FLAG_HAS_LOCATION) != 0); +}} + +/* is_destination(): 1 if cmd is a waypoint destination, 0 if not, -1 if unknown. */ +static inline int is_destination(uint16_t cmd) +{{ +\tconst int f = cmd_flags(cmd); +\treturn f < 0 ? -1 : ((f & (int)CMD_FLAG_IS_DESTINATION) != 0); +}} + +/* _bound_is_set(): non-zero if v is a finite bound (not NaN / ±Inf). + * Used internally by check_range(); bit-manipulation avoids isnan(). */ +static inline int _bound_is_set(float v) +{{ +\tuint32_t bits; +\t__builtin_memcpy(&bits, &v, sizeof(bits)); +\treturn (bits & 0x7F800000u) != 0x7F800000u; +}} + +/* check_range(): validate params against XML-defined and implicit bounds. + * + * Returns: + * 0 all params in range (or sentinel) + * 1-7 1-based index of the first param that fails a range check + * -1 command has no range data (not in table; validation not applied) + * + * is_int_frame=1 MISSION_ITEM_INT, or COMMAND_INT with global frame + * (p5/p6 are raw int32 values stored as float, scale 1e7 degrees). + * is_int_frame=0 COMMAND_LONG, or COMMAND_INT after the receiver has + * applied 1e7 or 1e4 scaling (p5/p6 are float degrees or metres). + * Pass 0.0f for p5/p6 to suppress the geographic range check (e.g. for + * COMMAND_INT with a local frame where p5/p6 are metres not degrees). */ +static inline int check_range(uint16_t cmd, int is_int_frame, +\tfloat p1, float p2, float p3, float p4, +\tfloat p5, float p6, float p7) +{{ +\tconst float params[7] = {{p1, p2, p3, p4, p5, p6, p7}}; +\tint found = 0; + +\t/* Binary search: find any entry for this cmd in param_bounds. */ +\tunsigned lo = 0u, hi = param_bounds_count; +\twhile (lo < hi) {{ +\t\tconst unsigned mid = lo + (hi - lo) / 2u; +\t\tif (param_bounds[mid].cmd < cmd) {{ lo = mid + 1u; continue; }} +\t\telse if (param_bounds[mid].cmd > cmd) {{ hi = mid; continue; }} +\t\t/* Found. Back up to first entry for this cmd. */ +\t\tunsigned start = mid; +\t\twhile (start > 0u && param_bounds[start - 1u].cmd == cmd) {{ --start; }} +\t\t/* Walk all entries for this cmd. */ +\t\tfor (unsigned i = start; +\t\t i < param_bounds_count && param_bounds[i].cmd == cmd; ++i) {{ +\t\t\tfound = 1; +\t\t\tconst unsigned pidx = (unsigned)param_bounds[i].param - 1u; +\t\t\tconst float v = params[pidx]; +\t\t\tif (is_sentinel(v)) {{ continue; }} +\t\t\tif (_bound_is_set(param_bounds[i].lo) && v < param_bounds[i].lo) +\t\t\t\t{{ return (int)param_bounds[i].param; }} +\t\t\tif (_bound_is_set(param_bounds[i].hi) && v > param_bounds[i].hi) +\t\t\t\t{{ return (int)param_bounds[i].param; }} +\t\t}} +\t\tbreak; +\t}} + +\t/* Implicit lat/lon range for location-bearing commands. */ +\tif (has_location(cmd) == 1) {{ +\t\tif (!lat_in_range(p5, is_int_frame)) {{ return 5; }} +\t\tif (!lon_in_range(p6, is_int_frame)) {{ return 6; }} +\t\tfound = 1; +\t}} + +\treturn found ? 0 : -1; +}} + +#ifdef __cplusplus +}} /* namespace mavlink_cmd_helpers */ +#endif +""" + + +def _write(out, bounds, flags): + # We need the xml basename for the comment; use a placeholder resolved at call time. + # Instead, just use a generic note. + out.write(_HEADER_TOP.format(xml="common.xml (via dialect include chain)")) + + # --- param_bounds table --- + out.write("/* Explicit param bounds from XML minValue / maxValue.\n" + " * (float)NAN for lo or hi means that side is unbounded. */\n") + out.write("struct ParamBound { uint16_t cmd; uint8_t param; float lo; float hi; };\n\n") + out.write("static const struct ParamBound param_bounds[] = {\n") + for cmd, pidx, lo, hi in bounds: + out.write( + "\t{{ {:5d}, {:d}, {:>14s}, {:>14s} }},\n".format( + cmd, pidx, float_c(lo), float_c(hi) + ) + ) + out.write("};\n") + out.write("static const unsigned param_bounds_count =\n" + "\tsizeof(param_bounds) / sizeof(param_bounds[0]);\n") + + out.write(_TABLES_MID.format()) + + # --- cmd_flags_table --- + out.write("static const struct CmdFlags cmd_flags_table[] = {\n") + for cmd, f, short in flags: + parts = [] + if f & 1: + parts.append("CMD_FLAG_HAS_LOCATION") + if f & 2: + parts.append("CMD_FLAG_IS_DESTINATION") + flag_str = " | ".join(parts) if parts else "0" + out.write("\t{{ {:5d}, {:s} }}, /* {:s} */\n".format(cmd, flag_str, short)) + out.write("};\n") + out.write("static const unsigned cmd_flags_count =\n" + "\tsizeof(cmd_flags_table) / sizeof(cmd_flags_table[0]);\n") + + out.write(_HEADER_BOTTOM.format()) + + +if __name__ == "__main__": + parser = argparse.ArgumentParser( + description="Generate MAVLink command range-check helper tables" + ) + parser.add_argument("xml", help="Root MAVLink XML dialect file (e.g. common.xml)") + parser.add_argument("--output", required=True, help="Output .h file path") + args = parser.parse_args() + generate(args.xml, args.output) + print("Generated", args.output) diff --git a/generator/mavparse.py b/generator/mavparse.py index 64356aa8d..14e6732c6 100644 --- a/generator/mavparse.py +++ b/generator/mavparse.py @@ -171,7 +171,7 @@ def set_description(self, description): self.description = description class MAVEnumEntry(object): - def __init__(self, name, value, description='', wip=False, end_marker=False, autovalue=False, origin_file='', origin_line=0, has_location=False): + def __init__(self, name, value, description='', wip=False, end_marker=False, autovalue=False, origin_file='', origin_line=0, has_location=False, is_destination=False): self.name = name self.value = value self.deprecated = None @@ -183,6 +183,7 @@ def __init__(self, name, value, description='', wip=False, end_marker=False, aut self.origin_file = origin_file self.origin_line = origin_line self.has_location = has_location + self.is_destination = is_destination class MAVEnum(object): def __init__(self, name, linenumber, description='', bitmask=False): @@ -308,6 +309,14 @@ def start_element(name, attrs): if type(has_location) != bool: raise MAVParseError("invalid has_location value %s" % has_location) + is_destination = attrs.get('isDestination', False) + if is_destination == 'true': + is_destination = True + elif is_destination == 'false': + is_destination = False + if type(is_destination) != bool: + raise MAVParseError("invalid is_destination value %s" % is_destination) + # check bitmask value if self.enum[-1].bitmask: # values should always be a power of 2. Py3.10 @@ -316,7 +325,7 @@ def start_element(name, attrs): print(f"{attrs['name']} has invalid values (bitmask must have powers of 2)") # append the new entry - self.enum[-1].entry.append(MAVEnumEntry(attrs['name'], value, '', False, False, autovalue, self.filename, p.CurrentLineNumber, has_location=has_location)) + self.enum[-1].entry.append(MAVEnumEntry(attrs['name'], value, '', False, False, autovalue, self.filename, p.CurrentLineNumber, has_location=has_location, is_destination=is_destination)) elif in_element == "mavlink.enums.enum.entry.wip": self.enum[-1].entry[-1].wip = True elif in_element == "mavlink.enums.enum.entry.param": From 1599e2bc4dcf495e7d4a9374be063db43e516736 Mon Sep 17 00:00:00 2001 From: Hamish Willee Date: Sat, 6 Jun 2026 12:59:53 +1000 Subject: [PATCH 02/14] Dumb Claude --- generator/mavgen_c_cmd_helpers.py | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/generator/mavgen_c_cmd_helpers.py b/generator/mavgen_c_cmd_helpers.py index 8f1a452ec..a54c5ac48 100644 --- a/generator/mavgen_c_cmd_helpers.py +++ b/generator/mavgen_c_cmd_helpers.py @@ -7,8 +7,7 @@ - Helper functions: is_sentinel, lat_in_range, lon_in_range, cmd_flags, has_location, is_destination, check_range -Only commands in SUPPORTED_CMDS (matching SupportedCommandParams in -mavlink_command_params.h) are included in the tables. +All commands with bounds or flags defined in the XML are included in the tables. """ import argparse @@ -28,13 +27,6 @@ from pymavlink.generator import mavparse -# Must match SupportedCommandParams[] in src/modules/mavlink/mavlink_command_params.h -SUPPORTED_CMDS = { - 16, 17, 19, 20, 21, 22, 31, 80, 84, 85, 93, 112, 114, 176, 177, 178, 179, 189, - 195, 196, 197, 201, 206, 211, 212, 214, 400, 420, 530, 532, 534, 2000, 2001, - 2003, 2500, 2501, 3000, 4501, 5000, 5001, 5002, 5003, 5004, 5100, 42600, -} - def load_with_includes(xml_path): """Parse xml_path and all transitively included files, merge enums into root.""" @@ -76,11 +68,8 @@ def generate(xml_path, output_path): if mav_cmd is None: raise RuntimeError("MAV_CMD enum not found in %s" % xml_path) - # Collect entries for supported commands, sorted by cmd value - entries = sorted( - (e for e in mav_cmd.entry if int(e.value) in SUPPORTED_CMDS), - key=lambda e: int(e.value), - ) + # Collect all MAV_CMD entries, sorted by cmd value + entries = sorted(mav_cmd.entry, key=lambda e: int(e.value)) # Build param bounds table — only params with at least one bound defined bounds = [] # list of (cmd, param_1based, lo_float, hi_float) From 644332b71f209535591480cadb5d2a46ee2f84a6 Mon Sep 17 00:00:00 2001 From: Hamish Willee Date: Sat, 6 Jun 2026 16:46:55 +1000 Subject: [PATCH 03/14] separate out range checking --- docs/cmd_range_checking.md | 176 ++++++++++++++++++++++++++++++ generator/mavgen_c_cmd_helpers.py | 57 ++++------ 2 files changed, 198 insertions(+), 35 deletions(-) create mode 100644 docs/cmd_range_checking.md diff --git a/docs/cmd_range_checking.md b/docs/cmd_range_checking.md new file mode 100644 index 000000000..635865b45 --- /dev/null +++ b/docs/cmd_range_checking.md @@ -0,0 +1,176 @@ +# MAV_CMD parameter range checking (`mavlink_cmd_helpers.h`) + +`mavlink_cmd_helpers.h` is a generated C header that provides O(log n) range validation for MAVLink command parameters. +It is produced by [`generator/mavgen_c_cmd_helpers.py`](../generator/mavgen_c_cmd_helpers.py) from the MAVLink XML `minValue`/`maxValue` attributes and the `hasLocation`/`isDestination` per-command flags. + +## Generating the header + +```sh +python3 generator/mavgen_c_cmd_helpers.py \ + message_definitions/v1.0/common.xml \ + --output path/to/mavlink_cmd_helpers.h +``` + +Include the result directly — it is a single self-contained `#pragma once` header with no library dependencies (only `` and ``). + +--- + +## API + +| Function | Description | +|---|---| +| `check_range(cmd, p1…p7)` | Validate all seven params against XML-defined bounds. Returns `0` (ok, including when the command has no range data), or `1–7` (1-based index of first failing param). Geographic range is **not** checked here. | +| `lat_in_range(v, is_int)` | `1` if `v` is a valid latitude (or sentinel), `0` if out of range. Optional — call when you want to catch out-of-range coordinates before passing to the flight stack. | +| `lon_in_range(v, is_int)` | `1` if `v` is a valid longitude (or sentinel), `0` if out of range. Optional — same conditions as `lat_in_range`. | +| `has_location(cmd)` | `1` if the command carries lat/lon/alt, `0` if not, `-1` if unknown. | +| `is_destination(cmd)` | `1` if the command is a waypoint destination, `0` if not, `-1` if unknown. | +| `is_sentinel(v)` | Non-zero if `v` is a "not provided" value (NaN or ±0.0 by default). Sentinels are skipped by `check_range`. | + +### `is_int` — coordinate-type flag for `lat_in_range` / `lon_in_range` + +`MISSION_ITEM_INT` and `COMMAND_INT` encode p5/p6 as `int32_t` multiplied by 1×10⁷. +`COMMAND_LONG` uses plain `float` degrees. +Pass `is_int` to select the appropriate range: + +| Message type | `is_int` | Valid lat range | Valid lon range | +|---|---|---|---| +| `COMMAND_INT` / `MISSION_ITEM_INT` | `true` | ±9×10⁸ (±90° × 1e7) | ±1.8×10⁹ (±180° × 1e7) | +| `COMMAND_LONG` | `false` | ±90° | ±180° | + +--- + +## Example: `MAV_CMD_DO_SET_HOME` (179) + +This command illustrates all three validation paths: + +| Param | Label | XML bounds | How `check_range` treats it | +|---|---|---|---| +| 1 | Use Current | *(none)* | no XML bound — passes unless sentinel | +| 2 | Roll | −180 … 180 °| explicit bounds from XML | +| 3 | Pitch | −90 … 90 ° | explicit bounds from XML | +| 4 | Yaw | −180 … 180 ° | explicit bounds from XML | +| 5 | Latitude | *(none in XML)* | implicit geographic range via `hasLocation="true"` | +| 6 | Longitude | *(none in XML)* | implicit geographic range via `hasLocation="true"` | +| 7 | Altitude | *(none)* | no XML bound — always passes | + +In the generated table this looks like: + +```c +// param_bounds[] +{ 179, 2, -180.0f, 180.0f }, // Roll +{ 179, 3, -90.0f, 90.0f }, // Pitch +{ 179, 4, -180.0f, 180.0f }, // Yaw + +// cmd_flags_table[] +{ 179, CMD_FLAG_HAS_LOCATION }, // DO_SET_HOME +``` + +--- + +### Unified handler (`COMMAND_INT`, `MISSION_ITEM_INT`, and `COMMAND_LONG`) + +In practice both PX4 and ArduPilot normalise all three message types into a single command handler before range-checking. +The caller casts `int32_t` x/y fields to `float` and passes `is_int` to select the correct coordinate scale: +`int32_t × 1e7` values span ±9×10⁸/±1.8×10⁹, while float-degree values span ±90/±180. +`is_int` also enables the INT32\_MAX sentinel check inside `is_coord_sentinel`. + +```c +#include "mavlink_cmd_helpers.h" + +void handle_command_both(mavlink_channel_t chan, + uint8_t sender_sysid, uint8_t sender_compid, + uint16_t command, + float p1, float p2, float p3, float p4, + float p5, float p6, float p7, + bool is_int, uint8_t frame) +{ + int r = check_range(command, p1, p2, p3, p4, p5, p6, p7); + + // Geographic check is optional — only needed when the receiver cares + // about catching out-of-range lat/lon before passing to the flight stack. + if (r == 0 && has_location(command) == 1) { + // COMMAND_LONG has no frame field so the check always applies. + // For INT types, restrict to global frames where p5/p6 are lat/lon. + bool check_geo = !is_int; + if (is_int) { + check_geo = (frame == MAV_FRAME_GLOBAL || + frame == MAV_FRAME_GLOBAL_INT || + frame == MAV_FRAME_GLOBAL_RELATIVE_ALT || + frame == MAV_FRAME_GLOBAL_RELATIVE_ALT_INT); + } + if (check_geo) { + if (!lat_in_range(p5, is_int)) r = 5; + else if (!lon_in_range(p6, is_int)) r = 6; + } + } + + if (r > 0) { + // r = 1-7: NACK with the 1-based index of the failing param. + mavlink_msg_command_ack_send(chan, command, MAV_RESULT_DENIED, + /*progress=*/0, /*result_param2=*/r, + sender_sysid, sender_compid); + return; + } + + // r == 0: params valid (or no range data for this command) — accept. +} +``` + +Call site for each message type: + +```c +/* COMMAND_INT or MISSION_ITEM_INT — x/y are int32_t × 1e7: */ +handle_command_both(chan, sysid, compid, msg->command, + msg->param1, msg->param2, msg->param3, msg->param4, + (float)msg->x, (float)msg->y, msg->z, + /*is_int=*/true, msg->frame); + +/* COMMAND_LONG — params are float degrees, no frame field: */ +handle_command_both(chan, sysid, compid, msg->command, + msg->param1, msg->param2, msg->param3, msg->param4, + msg->param5, msg->param6, msg->param7, + /*is_int=*/false, /*frame=*/0); +``` + +For `MAV_CMD_DO_SET_HOME` (179) with `MAV_FRAME_GLOBAL` and INT-scaled coordinates: + +| Scenario | p2 | p3 | p4 | p5 (`x`) | p6 (`y`) | `r` | +|---|---|---|---|---|---|---| +| Roll out of range | 200.0 | 0.0 | 0.0 | 473000000 | 85000000 | **2** | +| Pitch out of range | 45.0 | -100.0 | 0.0 | 473000000 | 85000000 | **3** | +| Longitude out of range | 45.0 | 30.0 | -90.0 | 473000000 | 1900000000 | **6** | +| All valid | 45.0 | 30.0 | -90.0 | 473000000 | 85000000 | **0** | +| All sentinels (NaN) | NaN | NaN | NaN | NaN | NaN | **0** | + +*(p5 = 473000000 = 47.3°N × 1e7, p6 = 85000000 = 8.5°E × 1e7, p6 = 1900000000 = 190° × 1e7 — out of the ±180° × 1e7 valid range. +Note: values ≥ 2×10⁹ are treated as an INT32\_MAX sentinel and skipped rather than rejected — use a value in (1.8×10⁹, 2.0×10⁹) to trigger a longitude rejection.)* + +For `MAV_CMD_DO_SET_HOME` (179) with float params p5/p6 in degrees (`COMMAND_LONG`): + +| Scenario | p2 | p3 | p4 | p5 (lat °) | p6 (lon °) | `r` | +|---|---|---|---|---|---|---| +| Roll out of range | 200.0 | 0.0 | 0.0 | 47.3 | 8.5 | **2** | +| Pitch out of range | 45.0 | -100.0 | 0.0 | 47.3 | 8.5 | **3** | +| Longitude out of range | 45.0 | 30.0 | -90.0 | 47.3 | 200.0 | **6** | +| All valid | 45.0 | 30.0 | -90.0 | 47.3 | 8.5 | **0** | + +--- + +## Unknown / custom commands + +If the command ID is not found in either `param_bounds` or `cmd_flags_table`, `check_range` returns `0` — the command is accepted. +Range checking simply does not apply to commands with no XML range data, so there is nothing to reject. +This covers vendor-specific commands, future commands not yet in the XML, and any standard command whose params have no `minValue`/`maxValue` attributes and whose `hasLocation` is false. + +--- + +## C++ usage + +The header wraps everything in `namespace mavlink_cmd_helpers` when compiled as C++: + +```cpp +#include "mavlink_cmd_helpers.h" + +int r = mavlink_cmd_helpers::check_range(cmd, p1, p2, p3, p4, p5, p6, p7); +bool loc = mavlink_cmd_helpers::has_location(cmd) == 1; +``` diff --git a/generator/mavgen_c_cmd_helpers.py b/generator/mavgen_c_cmd_helpers.py index a54c5ac48..b2dc6ea0b 100644 --- a/generator/mavgen_c_cmd_helpers.py +++ b/generator/mavgen_c_cmd_helpers.py @@ -108,6 +108,7 @@ def generate(xml_path, output_path): #pragma once #include +#include #include #ifdef __cplusplus @@ -140,34 +141,33 @@ def generate(xml_path, output_path): /* is_coord_sentinel(): additionally treats INT32_MAX-derived values as * sentinel. MISSION_ITEM_INT / COMMAND_INT use INT32_MAX (≈ 2.15e9 * as float) in params 5/6 to mean "use current position". */ -static inline int is_coord_sentinel(float v, int is_int_frame) +static inline int is_coord_sentinel(float v, bool is_int) {{ -\treturn is_sentinel(v) || (is_int_frame && (v >= 2.0e9f || v <= -2.0e9f)); +\treturn is_sentinel(v) || (is_int && (v >= 2.0e9f || v <= -2.0e9f)); }} /* ------------------------------------------------------------------- - * Implicit geographic range helpers + * Geographic range helpers — call these directly after check_range() + * when you know the frame is global and has_location(cmd) == 1. * - * is_int_frame=1: raw int32 (MISSION_ITEM_INT, or COMMAND_INT global - * before /1e7 conversion in handle_message_command_int). + * is_int=true: COMMAND_INT / MISSION_ITEM_INT — p5/p6 are int32 × 1e7. * lat ∈ [−9×10⁸, 9×10⁸] (±90° × 1e7) * lon ∈ [−1.8×10⁹, 1.8×10⁹] (±180° × 1e7) - * is_int_frame=0: float degrees (COMMAND_LONG, or COMMAND_INT after - * the receiver has applied the 1e7 scale). + * is_int=false: COMMAND_LONG — p5/p6 are float degrees. * lat ∈ [−90, 90], lon ∈ [−180, 180] * ------------------------------------------------------------------- */ -static inline int lat_in_range(float v, int is_int_frame) +static inline int lat_in_range(float v, bool is_int) {{ -\tif (is_coord_sentinel(v, is_int_frame)) return 1; -\treturn is_int_frame ? (v >= -9e8f && v <= 9e8f) -\t : (v >= -90.0f && v <= 90.0f); +\tif (is_coord_sentinel(v, is_int)) return 1; +\treturn is_int ? (v >= -9e8f && v <= 9e8f) +\t : (v >= -90.0f && v <= 90.0f); }} -static inline int lon_in_range(float v, int is_int_frame) +static inline int lon_in_range(float v, bool is_int) {{ -\tif (is_coord_sentinel(v, is_int_frame)) return 1; -\treturn is_int_frame ? (v >= -1.8e9f && v <= 1.8e9f) -\t : (v >= -180.0f && v <= 180.0f); +\tif (is_coord_sentinel(v, is_int)) return 1; +\treturn is_int ? (v >= -1.8e9f && v <= 1.8e9f) +\t : (v >= -180.0f && v <= 180.0f); }} """ @@ -225,25 +225,20 @@ def generate(xml_path, output_path): \treturn (bits & 0x7F800000u) != 0x7F800000u; }} -/* check_range(): validate params against XML-defined and implicit bounds. +/* check_range(): validate params 1-7 against XML-defined bounds only. * * Returns: - * 0 all params in range (or sentinel) + * 0 all params in range (or sentinel); also returned when the command + * has no range data — unknown commands are accepted, not rejected. * 1-7 1-based index of the first param that fails a range check - * -1 command has no range data (not in table; validation not applied) * - * is_int_frame=1 MISSION_ITEM_INT, or COMMAND_INT with global frame - * (p5/p6 are raw int32 values stored as float, scale 1e7 degrees). - * is_int_frame=0 COMMAND_LONG, or COMMAND_INT after the receiver has - * applied 1e7 or 1e4 scaling (p5/p6 are float degrees or metres). - * Pass 0.0f for p5/p6 to suppress the geographic range check (e.g. for - * COMMAND_INT with a local frame where p5/p6 are metres not degrees). */ -static inline int check_range(uint16_t cmd, int is_int_frame, + * Geographic range (lat/lon) is NOT checked here — call lat_in_range() and + * lon_in_range() separately when you know the frame is global. */ +static inline int check_range(uint16_t cmd, \tfloat p1, float p2, float p3, float p4, \tfloat p5, float p6, float p7) {{ \tconst float params[7] = {{p1, p2, p3, p4, p5, p6, p7}}; -\tint found = 0; \t/* Binary search: find any entry for this cmd in param_bounds. */ \tunsigned lo = 0u, hi = param_bounds_count; @@ -257,7 +252,6 @@ def generate(xml_path, output_path): \t\t/* Walk all entries for this cmd. */ \t\tfor (unsigned i = start; \t\t i < param_bounds_count && param_bounds[i].cmd == cmd; ++i) {{ -\t\t\tfound = 1; \t\t\tconst unsigned pidx = (unsigned)param_bounds[i].param - 1u; \t\t\tconst float v = params[pidx]; \t\t\tif (is_sentinel(v)) {{ continue; }} @@ -269,14 +263,7 @@ def generate(xml_path, output_path): \t\tbreak; \t}} -\t/* Implicit lat/lon range for location-bearing commands. */ -\tif (has_location(cmd) == 1) {{ -\t\tif (!lat_in_range(p5, is_int_frame)) {{ return 5; }} -\t\tif (!lon_in_range(p6, is_int_frame)) {{ return 6; }} -\t\tfound = 1; -\t}} - -\treturn found ? 0 : -1; +\treturn 0; }} #ifdef __cplusplus From 1d917fd527e54abd3b88f8cce0dc0e7aab245422 Mon Sep 17 00:00:00 2001 From: Hamish Willee Date: Sat, 6 Jun 2026 17:25:00 +1000 Subject: [PATCH 04/14] Make normal mavgen put the file in the right place --- generator/mavgen_c.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/generator/mavgen_c.py b/generator/mavgen_c.py index c20646fcb..90291f11d 100644 --- a/generator/mavgen_c.py +++ b/generator/mavgen_c.py @@ -7,7 +7,7 @@ ''' import os -from . import mavparse, mavtemplate +from . import mavparse, mavtemplate, mavgen_c_cmd_helpers t = mavtemplate.MAVTemplate() @@ -766,6 +766,9 @@ def generate(basename, xml_list): for idx in range(len(xml_list)): xml = xml_list[idx] - xml.xml_hash = hash(xml.basename) + xml.xml_hash = hash(xml.basename) generate_one(basename, xml) copy_fixed_headers(basename, xml_list[0]) + output_path = os.path.join(basename, "mavlink_cmd_helpers.h") + mavgen_c_cmd_helpers.generate(xml_list[0].filename, output_path) + print("Generated %s" % output_path) From fd7277ab692681f85817c57a44b70462f2f8a627 Mon Sep 17 00:00:00 2001 From: Hamish Willee Date: Wed, 10 Jun 2026 17:32:28 +1000 Subject: [PATCH 05/14] Simplify docs and API --- docs/cmd_range_checking.md | 221 ++++++++++-------------------- generator/mavgen_c.py | 7 +- generator/mavgen_c_cmd_helpers.py | 145 +++++++++++--------- 3 files changed, 156 insertions(+), 217 deletions(-) diff --git a/docs/cmd_range_checking.md b/docs/cmd_range_checking.md index 635865b45..3aa69e508 100644 --- a/docs/cmd_range_checking.md +++ b/docs/cmd_range_checking.md @@ -1,176 +1,105 @@ -# MAV_CMD parameter range checking (`mavlink_cmd_helpers.h`) +# MAV_CMD parameter range checking (`mav_cmd_helpers.h`) -`mavlink_cmd_helpers.h` is a generated C header that provides O(log n) range validation for MAVLink command parameters. -It is produced by [`generator/mavgen_c_cmd_helpers.py`](../generator/mavgen_c_cmd_helpers.py) from the MAVLink XML `minValue`/`maxValue` attributes and the `hasLocation`/`isDestination` per-command flags. +MAVLink command parameters (p1–p7) carry numeric values whose valid ranges are defined in the MAVLink XML — for example, a heading param may only accept 0–360°, or a speed param may require a non-negative value. +Validating these ranges at the MAVLink layer lets a vehicle or GCS reject malformed commands an mission items early, before they reach the flight stack, reducing the risk of undefined behaviour caused by out-of-range inputs. + +This PR provides efficient range checking in a generated mavgen_c header `mav_cmd_helpers.h`. +These can be used by any flight stack to check inputs. + +In addition to explicit range checking, it also provides methods for checking that lat/lon values are in valid ranges. ## Generating the header +The header is generated using mavgen as normal. + ```sh -python3 generator/mavgen_c_cmd_helpers.py \ - message_definitions/v1.0/common.xml \ - --output path/to/mavlink_cmd_helpers.h +python3 generator/mavgen.py \ + --lang C \ + --output /path/to/output \ + message_definitions/v1.0/common.xml ``` -Include the result directly — it is a single self-contained `#pragma once` header with no library dependencies (only `` and ``). - ---- +The `mav_cmd_helpers.h` in this case would be generated to the root of each of the generated dialect folders. +Include the result directly — it is a single self-contained `#pragma once` header with no library dependencies (only ``, ``, and ``). ## API -| Function | Description | -|---|---| -| `check_range(cmd, p1…p7)` | Validate all seven params against XML-defined bounds. Returns `0` (ok, including when the command has no range data), or `1–7` (1-based index of first failing param). Geographic range is **not** checked here. | -| `lat_in_range(v, is_int)` | `1` if `v` is a valid latitude (or sentinel), `0` if out of range. Optional — call when you want to catch out-of-range coordinates before passing to the flight stack. | -| `lon_in_range(v, is_int)` | `1` if `v` is a valid longitude (or sentinel), `0` if out of range. Optional — same conditions as `lat_in_range`. | -| `has_location(cmd)` | `1` if the command carries lat/lon/alt, `0` if not, `-1` if unknown. | -| `is_destination(cmd)` | `1` if the command is a waypoint destination, `0` if not, `-1` if unknown. | -| `is_sentinel(v)` | Non-zero if `v` is a "not provided" value (NaN or ±0.0 by default). Sentinels are skipped by `check_range`. | - -### `is_int` — coordinate-type flag for `lat_in_range` / `lon_in_range` - -`MISSION_ITEM_INT` and `COMMAND_INT` encode p5/p6 as `int32_t` multiplied by 1×10⁷. -`COMMAND_LONG` uses plain `float` degrees. -Pass `is_int` to select the appropriate range: - -| Message type | `is_int` | Valid lat range | Valid lon range | -|---|---|---|---| -| `COMMAND_INT` / `MISSION_ITEM_INT` | `true` | ±9×10⁸ (±90° × 1e7) | ±1.8×10⁹ (±180° × 1e7) | -| `COMMAND_LONG` | `false` | ±90° | ±180° | - ---- - -## Example: `MAV_CMD_DO_SET_HOME` (179) +| Function | Description | +| ---------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `check_range(cmd, p1…p7)` | Validate all seven params against XML-defined bounds. Returns `0` (ok, including when the command has no range data), or `1–7` (1-based index of first failing param). Geographic range is **not** checked here. | +| `lat_in_range(lat, is_int)` | `1` if `lat` is in range for a latitude, `0` if out of range or NaN. Call after `param_invalid` to check coordinates. | +| `lon_in_range(lon, is_int)` | `1` if `lon` is in range for a longitude, `0` if out of range or NaN. Same conditions as `lat_in_range`. | +| `has_location(cmd)` | `1` if the command carries lat/lon/alt, `0` if not, `-1` if unknown. | +| `is_destination(cmd)` | `1` if the command is a waypoint destination, `0` if not, `-1` if unknown. | +| `param_invalid(param_val, is_int)` | Non-zero if `param_val` is an invalid/default value (NaN or ±0.0 by default; also INT32_MAX-scale when `is_int` is true). Invalid params are skipped by `check_range` (called with `is_int=false`). | -This command illustrates all three validation paths: +### Using the methods -| Param | Label | XML bounds | How `check_range` treats it | -|---|---|---|---| -| 1 | Use Current | *(none)* | no XML bound — passes unless sentinel | -| 2 | Roll | −180 … 180 °| explicit bounds from XML | -| 3 | Pitch | −90 … 90 ° | explicit bounds from XML | -| 4 | Yaw | −180 … 180 ° | explicit bounds from XML | -| 5 | Latitude | *(none in XML)* | implicit geographic range via `hasLocation="true"` | -| 6 | Longitude | *(none in XML)* | implicit geographic range via `hasLocation="true"` | -| 7 | Altitude | *(none)* | no XML bound — always passes | +The methods are intended to be used in command handlers and during mission upload to reject MAV_CMD with passed values that are out of range. +The `check_range()` method returns `0` if all the passed params are all in range, have no range, or are the sentinel values - NaN/0/INT32MAX-for-param5or6, and otherwise returns the value of the first out of range param. +The `lat_in_range()` and `lon_in_range()` can further be used to check that lat/lon values aren't passed ranges that are bigger than valid lat/lon values. -In the generated table this looks like: +This code shows how you might check a mission item. +The result is either 0, or the param number of the first out of range param. ```c -// param_bounds[] -{ 179, 2, -180.0f, 180.0f }, // Roll -{ 179, 3, -90.0f, 90.0f }, // Pitch -{ 179, 4, -180.0f, 180.0f }, // Yaw +#include "common/mav_cmd_helpers.h" -// cmd_flags_table[] -{ 179, CMD_FLAG_HAS_LOCATION }, // DO_SET_HOME -``` - ---- +/* Returns the 1-based index of the first out-of-range param, or 0 if all ok. */ +int check_mission_item(const mavlink_mission_item_int_t *item) +{ + // check_range uses is_int=false internally, but that is fine: location params + // (x/y) have no XML bounds so are never evaluated; p1-p4/p7 are always float. + int r = check_range(item->command, + item->param1, item->param2, item->param3, item->param4, + (float)item->x, (float)item->y, item->z); + if (r != 0) return r; + + if (has_location(item->command) == 1) { + if (!param_invalid((float)item->x, true) && !lat_in_range((float)item->x, true)) return 5; + if (!param_invalid((float)item->y, true) && !lon_in_range((float)item->y, true)) return 6; + } -### Unified handler (`COMMAND_INT`, `MISSION_ITEM_INT`, and `COMMAND_LONG`) + return 0; +} +``` -In practice both PX4 and ArduPilot normalise all three message types into a single command handler before range-checking. -The caller casts `int32_t` x/y fields to `float` and passes `is_int` to select the correct coordinate scale: -`int32_t × 1e7` values span ±9×10⁸/±1.8×10⁹, while float-degree values span ±90/±180. -`is_int` also enables the INT32\_MAX sentinel check inside `is_coord_sentinel`. +For a `COMMAND_LONG` (all params are plain floats): ```c -#include "mavlink_cmd_helpers.h" - -void handle_command_both(mavlink_channel_t chan, - uint8_t sender_sysid, uint8_t sender_compid, - uint16_t command, - float p1, float p2, float p3, float p4, - float p5, float p6, float p7, - bool is_int, uint8_t frame) +/* Returns the 1-based index of the first out-of-range param, or 0 if all ok. */ +int check_command_long(const mavlink_command_long_t *cmd) { - int r = check_range(command, p1, p2, p3, p4, p5, p6, p7); - - // Geographic check is optional — only needed when the receiver cares - // about catching out-of-range lat/lon before passing to the flight stack. - if (r == 0 && has_location(command) == 1) { - // COMMAND_LONG has no frame field so the check always applies. - // For INT types, restrict to global frames where p5/p6 are lat/lon. - bool check_geo = !is_int; - if (is_int) { - check_geo = (frame == MAV_FRAME_GLOBAL || - frame == MAV_FRAME_GLOBAL_INT || - frame == MAV_FRAME_GLOBAL_RELATIVE_ALT || - frame == MAV_FRAME_GLOBAL_RELATIVE_ALT_INT); - } - if (check_geo) { - if (!lat_in_range(p5, is_int)) r = 5; - else if (!lon_in_range(p6, is_int)) r = 6; - } - } - - if (r > 0) { - // r = 1-7: NACK with the 1-based index of the failing param. - mavlink_msg_command_ack_send(chan, command, MAV_RESULT_DENIED, - /*progress=*/0, /*result_param2=*/r, - sender_sysid, sender_compid); - return; + int r = check_range(cmd->command, + cmd->param1, cmd->param2, cmd->param3, cmd->param4, + cmd->param5, cmd->param6, cmd->param7); + if (r != 0) return r; + + if (has_location(cmd->command) == 1) { + if (!param_invalid(cmd->param5, false) && !lat_in_range(cmd->param5, false)) return 5; + if (!param_invalid(cmd->param6, false) && !lon_in_range(cmd->param6, false)) return 6; } - // r == 0: params valid (or no range data for this command) — accept. + return 0; } ``` -Call site for each message type: +For a `COMMAND_INT` (x/y are `int32_t` × 1e7, cast to float with `is_int=true`): ```c -/* COMMAND_INT or MISSION_ITEM_INT — x/y are int32_t × 1e7: */ -handle_command_both(chan, sysid, compid, msg->command, - msg->param1, msg->param2, msg->param3, msg->param4, - (float)msg->x, (float)msg->y, msg->z, - /*is_int=*/true, msg->frame); - -/* COMMAND_LONG — params are float degrees, no frame field: */ -handle_command_both(chan, sysid, compid, msg->command, - msg->param1, msg->param2, msg->param3, msg->param4, - msg->param5, msg->param6, msg->param7, - /*is_int=*/false, /*frame=*/0); -``` - -For `MAV_CMD_DO_SET_HOME` (179) with `MAV_FRAME_GLOBAL` and INT-scaled coordinates: - -| Scenario | p2 | p3 | p4 | p5 (`x`) | p6 (`y`) | `r` | -|---|---|---|---|---|---|---| -| Roll out of range | 200.0 | 0.0 | 0.0 | 473000000 | 85000000 | **2** | -| Pitch out of range | 45.0 | -100.0 | 0.0 | 473000000 | 85000000 | **3** | -| Longitude out of range | 45.0 | 30.0 | -90.0 | 473000000 | 1900000000 | **6** | -| All valid | 45.0 | 30.0 | -90.0 | 473000000 | 85000000 | **0** | -| All sentinels (NaN) | NaN | NaN | NaN | NaN | NaN | **0** | - -*(p5 = 473000000 = 47.3°N × 1e7, p6 = 85000000 = 8.5°E × 1e7, p6 = 1900000000 = 190° × 1e7 — out of the ±180° × 1e7 valid range. -Note: values ≥ 2×10⁹ are treated as an INT32\_MAX sentinel and skipped rather than rejected — use a value in (1.8×10⁹, 2.0×10⁹) to trigger a longitude rejection.)* - -For `MAV_CMD_DO_SET_HOME` (179) with float params p5/p6 in degrees (`COMMAND_LONG`): - -| Scenario | p2 | p3 | p4 | p5 (lat °) | p6 (lon °) | `r` | -|---|---|---|---|---|---|---| -| Roll out of range | 200.0 | 0.0 | 0.0 | 47.3 | 8.5 | **2** | -| Pitch out of range | 45.0 | -100.0 | 0.0 | 47.3 | 8.5 | **3** | -| Longitude out of range | 45.0 | 30.0 | -90.0 | 47.3 | 200.0 | **6** | -| All valid | 45.0 | 30.0 | -90.0 | 47.3 | 8.5 | **0** | - ---- - -## Unknown / custom commands - -If the command ID is not found in either `param_bounds` or `cmd_flags_table`, `check_range` returns `0` — the command is accepted. -Range checking simply does not apply to commands with no XML range data, so there is nothing to reject. -This covers vendor-specific commands, future commands not yet in the XML, and any standard command whose params have no `minValue`/`maxValue` attributes and whose `hasLocation` is false. - ---- - -## C++ usage - -The header wraps everything in `namespace mavlink_cmd_helpers` when compiled as C++: - -```cpp -#include "mavlink_cmd_helpers.h" +/* Returns the 1-based index of the first out-of-range param, or 0 if all ok. */ +int check_command_int(const mavlink_command_int_t *cmd) +{ + int r = check_range(cmd->command, + cmd->param1, cmd->param2, cmd->param3, cmd->param4, + (float)cmd->x, (float)cmd->y, cmd->z); + if (r != 0) return r; + + if (has_location(cmd->command) == 1) { + if (!param_invalid((float)cmd->x, true) && !lat_in_range((float)cmd->x, true)) return 5; + if (!param_invalid((float)cmd->y, true) && !lon_in_range((float)cmd->y, true)) return 6; + } -int r = mavlink_cmd_helpers::check_range(cmd, p1, p2, p3, p4, p5, p6, p7); -bool loc = mavlink_cmd_helpers::has_location(cmd) == 1; + return 0; +} ``` diff --git a/generator/mavgen_c.py b/generator/mavgen_c.py index 90291f11d..981d1582b 100644 --- a/generator/mavgen_c.py +++ b/generator/mavgen_c.py @@ -768,7 +768,8 @@ def generate(basename, xml_list): xml = xml_list[idx] xml.xml_hash = hash(xml.basename) generate_one(basename, xml) + directory = os.path.join(basename, xml.basename) + output_path = os.path.join(directory, "mav_cmd_helpers.h") + mavgen_c_cmd_helpers.generate(xml.filename, output_path) + print("Generated %s" % output_path) copy_fixed_headers(basename, xml_list[0]) - output_path = os.path.join(basename, "mavlink_cmd_helpers.h") - mavgen_c_cmd_helpers.generate(xml_list[0].filename, output_path) - print("Generated %s" % output_path) diff --git a/generator/mavgen_c_cmd_helpers.py b/generator/mavgen_c_cmd_helpers.py index b2dc6ea0b..41a4e6d6a 100644 --- a/generator/mavgen_c_cmd_helpers.py +++ b/generator/mavgen_c_cmd_helpers.py @@ -1,10 +1,10 @@ #!/usr/bin/env python3 -"""Generate mavlink_cmd_helpers.h — per-MAV_CMD range-check tables and helpers. +"""Generate mav_cmd_helpers.h — per-MAV_CMD range-check tables and helpers. Outputs a header with: - param_bounds[] explicit min/max from XML minValue/maxValue attributes - cmd_flags_table[] hasLocation and isDestination flags from XML - - Helper functions: is_sentinel, lat_in_range, lon_in_range, + - Helper functions: is_float_param_invalid, lat_in_range, lon_in_range, cmd_flags, has_location, is_destination, check_range All commands with bounds or flags defined in the XML are included in the tables. @@ -112,62 +112,63 @@ def generate(xml_path, output_path): #include #ifdef __cplusplus -namespace mavlink_cmd_helpers {{ +namespace mav_cmd_helpers {{ #endif -/* ------------------------------------------------------------------- - * Sentinel detection +/** + * @brief Tests if a parameter value is set to an invalid/default value (NaN, possibly 0, or INT32_MAX-derived). * - * is_sentinel(): non-zero if v means "param not provided". - * NaN is the MAVLink standard sentinel. - * ±0.0 is also accepted by default: many GCS tools send 0 for unused - * fields rather than NaN. + * Returns non-zero if param_val is NaN or ±0.0, which are commonly used default values + * for float parameters that are not in use. + * When is_int is true (COMMAND_INT / MISSION_ITEM_INT encoding), also treats values with + * magnitude ≥ 2.0×10⁹ as invalid; INT32_MAX (≈ 2.15×10⁹) is used in p5/p6 to mean + * "use current position". The threshold is set below INT32_MAX but above the maximum + * valid coordinate (±1.8×10⁹). * - * Define MAVLINK_CMD_STRICT_SENTINEL (e.g. -DMAVLINK_CMD_STRICT_SENTINEL - * in a test build) to reject ±0.0 and require NaN only — useful for - * auditing senders that send 0 instead of NaN. - * ------------------------------------------------------------------- */ -static inline int is_sentinel(float v) + * Define MAV_CMD_STRICT_NAN_INVALID to reject ±0.0 and require NaN only. + * + * @param param_val Parameter value to test. + * @param is_int True for COMMAND_INT / MISSION_ITEM_INT encoding (activates INT32_MAX check). + * @return Non-zero if param_val is invalid, zero otherwise. + */ +static inline int param_invalid(float param_val, bool is_int) {{ \tuint32_t bits; -\t__builtin_memcpy(&bits, &v, sizeof(bits)); -\tif ((bits & 0x7F800000u) == 0x7F800000u) return 1; /* NaN — always a sentinel */ -#ifndef MAVLINK_CMD_STRICT_SENTINEL -\tif (!(bits & 0x7FFFFFFFu)) return 1; /* ±0.0 — default sentinel */ +\t__builtin_memcpy(&bits, ¶m_val, sizeof(bits)); +\tif ((bits & 0x7F800000u) == 0x7F800000u) return 1; /* NaN — always invalid */ +#ifndef MAV_CMD_STRICT_NAN_INVALID +\tif (!(bits & 0x7FFFFFFFu)) return 1; /* ±0.0 — invalid by default */ #endif +\tif (is_int && (param_val >= 2.0e9f || param_val <= -2.0e9f)) return 1; /* INT32_MAX — invalid */ \treturn 0; }} -/* is_coord_sentinel(): additionally treats INT32_MAX-derived values as - * sentinel. MISSION_ITEM_INT / COMMAND_INT use INT32_MAX (≈ 2.15e9 - * as float) in params 5/6 to mean "use current position". */ -static inline int is_coord_sentinel(float v, bool is_int) -{{ -\treturn is_sentinel(v) || (is_int && (v >= 2.0e9f || v <= -2.0e9f)); -}} - -/* ------------------------------------------------------------------- - * Geographic range helpers — call these directly after check_range() - * when you know the frame is global and has_location(cmd) == 1. +/** + * @brief Tests if lat is in range for a latitude. * - * is_int=true: COMMAND_INT / MISSION_ITEM_INT — p5/p6 are int32 × 1e7. - * lat ∈ [−9×10⁸, 9×10⁸] (±90° × 1e7) - * lon ∈ [−1.8×10⁹, 1.8×10⁹] (±180° × 1e7) - * is_int=false: COMMAND_LONG — p5/p6 are float degrees. - * lat ∈ [−90, 90], lon ∈ [−180, 180] - * ------------------------------------------------------------------- */ -static inline int lat_in_range(float v, bool is_int) + * @param lat Latitude to test. + * @param is_int True for COMMAND_INT / MISSION_ITEM_INT (int32 × 1e7, range ±9×10⁸); + * false for COMMAND_LONG (float degrees, range ±90). + * @return 1 if in range, 0 if out of range or NaN. + */ +static inline int lat_in_range(float lat, bool is_int) {{ -\tif (is_coord_sentinel(v, is_int)) return 1; -\treturn is_int ? (v >= -9e8f && v <= 9e8f) -\t : (v >= -90.0f && v <= 90.0f); +\treturn is_int ? (lat >= -9e8f && lat <= 9e8f) +\t : (lat >= -90.0f && lat <= 90.0f); }} -static inline int lon_in_range(float v, bool is_int) +/** + * @brief Tests if lon is in range for a longitude. + * + * @param lon Longitude to test. + * @param is_int True for COMMAND_INT / MISSION_ITEM_INT (int32 × 1e7, range ±1.8×10⁹); + * false for COMMAND_LONG (float degrees, range ±180). + * @return 1 if in range, 0 if out of range or NaN. + */ +static inline int lon_in_range(float lon, bool is_int) {{ -\tif (is_coord_sentinel(v, is_int)) return 1; -\treturn is_int ? (v >= -1.8e9f && v <= 1.8e9f) -\t : (v >= -180.0f && v <= 180.0f); +\treturn is_int ? (lon >= -1.8e9f && lon <= 1.8e9f) +\t : (lon >= -180.0f && lon <= 180.0f); }} """ @@ -184,12 +185,12 @@ def generate(xml_path, output_path): _HEADER_BOTTOM = """\ -/* ------------------------------------------------------------------- - * Public helpers - * ------------------------------------------------------------------- */ - -/* cmd_flags(): binary search for command attribute flags. - * Returns the flags byte, or -1 if cmd is not in the table. */ +/** + * @brief Binary-search lookup for per-command attribute flags. + * + * @param cmd MAV_CMD command ID. + * @return Flags byte (CMD_FLAG_HAS_LOCATION | CMD_FLAG_IS_DESTINATION), or -1 if unknown. + */ static inline int cmd_flags(uint16_t cmd) {{ \tunsigned lo = 0u, hi = cmd_flags_count; @@ -202,38 +203,46 @@ def generate(xml_path, output_path): \treturn -1; }} -/* has_location(): 1 if cmd carries lat/lon/alt, 0 if not, -1 if unknown. */ +/** + * @brief Test if a command carries lat/lon/alt. + * + * @param cmd MAV_CMD command ID. + * @return 1 if the command has location, 0 if not, -1 if unknown. + */ static inline int has_location(uint16_t cmd) {{ \tconst int f = cmd_flags(cmd); \treturn f < 0 ? -1 : ((f & (int)CMD_FLAG_HAS_LOCATION) != 0); }} -/* is_destination(): 1 if cmd is a waypoint destination, 0 if not, -1 if unknown. */ +/** + * @brief Test if a command is a waypoint destination. + * + * @param cmd MAV_CMD command ID. + * @return 1 if the command is a destination, 0 if not, -1 if unknown. + */ static inline int is_destination(uint16_t cmd) {{ \tconst int f = cmd_flags(cmd); \treturn f < 0 ? -1 : ((f & (int)CMD_FLAG_IS_DESTINATION) != 0); }} -/* _bound_is_set(): non-zero if v is a finite bound (not NaN / ±Inf). - * Used internally by check_range(); bit-manipulation avoids isnan(). */ -static inline int _bound_is_set(float v) +/* Internal: non-zero if bound is a finite value (not NaN / ±Inf). Bit-manipulation avoids isnan(). */ +static inline int _bound_is_set(float bound) {{ \tuint32_t bits; -\t__builtin_memcpy(&bits, &v, sizeof(bits)); +\t__builtin_memcpy(&bits, &bound, sizeof(bits)); \treturn (bits & 0x7F800000u) != 0x7F800000u; }} -/* check_range(): validate params 1-7 against XML-defined bounds only. +/** + * @brief Validate MAVLink command parameters against XML-defined bounds. * - * Returns: - * 0 all params in range (or sentinel); also returned when the command - * has no range data — unknown commands are accepted, not rejected. - * 1-7 1-based index of the first param that fails a range check - * - * Geographic range (lat/lon) is NOT checked here — call lat_in_range() and - * lon_in_range() separately when you know the frame is global. */ + * @param cmd MAV_CMD command ID. + * @param p1,p2,p3,p4,p5,p6,p7 Command parameters 1–7. + * @return 0 if all params are in range or param_invalid values (e.g. NaN); 1–7 (1-based index of + * the first failing parameter) otherwise. + */ static inline int check_range(uint16_t cmd, \tfloat p1, float p2, float p3, float p4, \tfloat p5, float p6, float p7) @@ -253,11 +262,11 @@ def generate(xml_path, output_path): \t\tfor (unsigned i = start; \t\t i < param_bounds_count && param_bounds[i].cmd == cmd; ++i) {{ \t\t\tconst unsigned pidx = (unsigned)param_bounds[i].param - 1u; -\t\t\tconst float v = params[pidx]; -\t\t\tif (is_sentinel(v)) {{ continue; }} -\t\t\tif (_bound_is_set(param_bounds[i].lo) && v < param_bounds[i].lo) +\t\t\tconst float param_val = params[pidx]; +\t\t\tif (param_invalid(param_val, false)) {{ continue; }} +\t\t\tif (_bound_is_set(param_bounds[i].lo) && param_val < param_bounds[i].lo) \t\t\t\t{{ return (int)param_bounds[i].param; }} -\t\t\tif (_bound_is_set(param_bounds[i].hi) && v > param_bounds[i].hi) +\t\t\tif (_bound_is_set(param_bounds[i].hi) && param_val > param_bounds[i].hi) \t\t\t\t{{ return (int)param_bounds[i].param; }} \t\t}} \t\tbreak; @@ -267,7 +276,7 @@ def generate(xml_path, output_path): }} #ifdef __cplusplus -}} /* namespace mavlink_cmd_helpers */ +}} /* namespace mav_cmd_helpers */ #endif """ From b99bd45b41f5c1a94862bc5c707614741339f75e Mon Sep 17 00:00:00 2001 From: Julian Oes Date: Wed, 24 Jun 2026 10:47:28 +1200 Subject: [PATCH 06/14] mavgen_c: skip cmd helpers for dialects without MAV_CMD Dialects such as standard.xml and minimal.xml define no MAV_CMD enum, so generating mav_cmd_helpers.h raised RuntimeError and aborted the whole mavgen run when more than one dialect was processed. Make mavgen_c_cmd_helpers.generate() return False and skip writing the header in that case instead of raising, and only print "Generated" when a header was actually emitted. --- generator/mavgen_c.py | 4 ++-- generator/mavgen_c_cmd_helpers.py | 13 ++++++++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/generator/mavgen_c.py b/generator/mavgen_c.py index 981d1582b..8d392c931 100644 --- a/generator/mavgen_c.py +++ b/generator/mavgen_c.py @@ -770,6 +770,6 @@ def generate(basename, xml_list): generate_one(basename, xml) directory = os.path.join(basename, xml.basename) output_path = os.path.join(directory, "mav_cmd_helpers.h") - mavgen_c_cmd_helpers.generate(xml.filename, output_path) - print("Generated %s" % output_path) + if mavgen_c_cmd_helpers.generate(xml.filename, output_path): + print("Generated %s" % output_path) copy_fixed_headers(basename, xml_list[0]) diff --git a/generator/mavgen_c_cmd_helpers.py b/generator/mavgen_c_cmd_helpers.py index 41a4e6d6a..f4f578d1e 100644 --- a/generator/mavgen_c_cmd_helpers.py +++ b/generator/mavgen_c_cmd_helpers.py @@ -66,7 +66,10 @@ def generate(xml_path, output_path): mav_cmd = next((e for e in root.enum if e.name == "MAV_CMD"), None) if mav_cmd is None: - raise RuntimeError("MAV_CMD enum not found in %s" % xml_path) + # This dialect (and its includes) defines no MAV_CMD enum, so there is + # nothing to range-check. Skip generating the header rather than failing + # the whole mavgen run. + return False # Collect all MAV_CMD entries, sorted by cmd value entries = sorted(mav_cmd.entry, key=lambda e: int(e.value)) @@ -100,6 +103,8 @@ def generate(xml_path, output_path): with open(output_path, "w") as out: _write(out, bounds, flags) + return True + _HEADER_TOP = """\ /* AUTO-GENERATED by mavgen_cmd_helpers.py from {xml} — do not edit. @@ -327,5 +332,7 @@ def _write(out, bounds, flags): parser.add_argument("xml", help="Root MAVLink XML dialect file (e.g. common.xml)") parser.add_argument("--output", required=True, help="Output .h file path") args = parser.parse_args() - generate(args.xml, args.output) - print("Generated", args.output) + if generate(args.xml, args.output): + print("Generated", args.output) + else: + print("Skipped %s: no MAV_CMD enum in %s" % (args.output, args.xml)) From 7a0087475b3219ee14b7ce69871341bf7e9b95b5 Mon Sep 17 00:00:00 2001 From: Julian Oes Date: Wed, 24 Jun 2026 11:05:20 +1200 Subject: [PATCH 07/14] mavgen_c: split is_int range helpers into typed int/float variants The bool is_int parameter on param_invalid/lat_in_range/lon_in_range was easy to pass wrong and forced lossy (float)int32 casts at call sites. Replace it with explicitly typed variants: param_invalid(float) NaN / +-0.0 coord_invalid_int(int32_t) INT32_MAX 'use current' sentinel lat_in_range_int(int32_t) lat_in_range_float(float) lon_in_range_int(int32_t) lon_in_range_float(float) The int variants take int32_t degE7 directly so COMMAND_INT / MISSION_ITEM_INT callers no longer cast. Update check_range and the docs examples accordingly. --- docs/cmd_range_checking.md | 40 +++++++++------- generator/mavgen_c_cmd_helpers.py | 78 ++++++++++++++++++++----------- 2 files changed, 73 insertions(+), 45 deletions(-) diff --git a/docs/cmd_range_checking.md b/docs/cmd_range_checking.md index 3aa69e508..579d0ee26 100644 --- a/docs/cmd_range_checking.md +++ b/docs/cmd_range_checking.md @@ -20,18 +20,21 @@ python3 generator/mavgen.py \ ``` The `mav_cmd_helpers.h` in this case would be generated to the root of each of the generated dialect folders. -Include the result directly — it is a single self-contained `#pragma once` header with no library dependencies (only ``, ``, and ``). +Include the result directly — it is a single self-contained `#pragma once` header with no library dependencies (only `` and ``). ## API -| Function | Description | -| ---------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `check_range(cmd, p1…p7)` | Validate all seven params against XML-defined bounds. Returns `0` (ok, including when the command has no range data), or `1–7` (1-based index of first failing param). Geographic range is **not** checked here. | -| `lat_in_range(lat, is_int)` | `1` if `lat` is in range for a latitude, `0` if out of range or NaN. Call after `param_invalid` to check coordinates. | -| `lon_in_range(lon, is_int)` | `1` if `lon` is in range for a longitude, `0` if out of range or NaN. Same conditions as `lat_in_range`. | -| `has_location(cmd)` | `1` if the command carries lat/lon/alt, `0` if not, `-1` if unknown. | -| `is_destination(cmd)` | `1` if the command is a waypoint destination, `0` if not, `-1` if unknown. | -| `param_invalid(param_val, is_int)` | Non-zero if `param_val` is an invalid/default value (NaN or ±0.0 by default; also INT32_MAX-scale when `is_int` is true). Invalid params are skipped by `check_range` (called with `is_int=false`). | +| Function | Description | +| ----------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `check_range(cmd, p1…p7)` | Validate all seven params against XML-defined bounds. Returns `0` (ok, including when the command has no range data), or `1–7` (1-based index of first failing param). Geographic range is **not** checked here. | +| `lat_in_range_int(lat)` | `1` if `lat` (int32 degE7, COMMAND_INT / MISSION_ITEM_INT) is in range for a latitude, else `0`. Call after `coord_invalid_int` to check coordinates. | +| `lat_in_range_float(lat)` | `1` if `lat` (float degrees, COMMAND_LONG) is in range for a latitude, `0` if out of range or NaN. Call after `param_invalid` to check coordinates. | +| `lon_in_range_int(lon)` | `1` if `lon` (int32 degE7) is in range for a longitude, else `0`. | +| `lon_in_range_float(lon)` | `1` if `lon` (float degrees) is in range for a longitude, `0` if out of range or NaN. | +| `has_location(cmd)` | `1` if the command carries lat/lon/alt, `0` if not, `-1` if unknown. | +| `is_destination(cmd)` | `1` if the command is a waypoint destination, `0` if not, `-1` if unknown. | +| `param_invalid(param_val)` | Non-zero if the float `param_val` is an invalid/default value (NaN or ±0.0 by default). Invalid params are skipped by `check_range`. | +| `coord_invalid_int(coord)` | Non-zero if the int32 degE7 `coord` is the `INT32_MAX` "use current position" sentinel. | ### Using the methods @@ -48,16 +51,17 @@ The result is either 0, or the param number of the first out of range param. /* Returns the 1-based index of the first out-of-range param, or 0 if all ok. */ int check_mission_item(const mavlink_mission_item_int_t *item) { - // check_range uses is_int=false internally, but that is fine: location params - // (x/y) have no XML bounds so are never evaluated; p1-p4/p7 are always float. + // Passing x/y as floats to check_range is fine: location params (x/y) have no + // XML bounds so are never evaluated there; p1-p4/p7 are always float anyway. + // The int32 lat/lon are range-checked separately below with the _int helpers. int r = check_range(item->command, item->param1, item->param2, item->param3, item->param4, (float)item->x, (float)item->y, item->z); if (r != 0) return r; if (has_location(item->command) == 1) { - if (!param_invalid((float)item->x, true) && !lat_in_range((float)item->x, true)) return 5; - if (!param_invalid((float)item->y, true) && !lon_in_range((float)item->y, true)) return 6; + if (!coord_invalid_int(item->x) && !lat_in_range_int(item->x)) return 5; + if (!coord_invalid_int(item->y) && !lon_in_range_int(item->y)) return 6; } return 0; @@ -76,15 +80,15 @@ int check_command_long(const mavlink_command_long_t *cmd) if (r != 0) return r; if (has_location(cmd->command) == 1) { - if (!param_invalid(cmd->param5, false) && !lat_in_range(cmd->param5, false)) return 5; - if (!param_invalid(cmd->param6, false) && !lon_in_range(cmd->param6, false)) return 6; + if (!param_invalid(cmd->param5) && !lat_in_range_float(cmd->param5)) return 5; + if (!param_invalid(cmd->param6) && !lon_in_range_float(cmd->param6)) return 6; } return 0; } ``` -For a `COMMAND_INT` (x/y are `int32_t` × 1e7, cast to float with `is_int=true`): +For a `COMMAND_INT` (x/y are `int32_t` × 1e7, checked directly with the `_int` helpers): ```c /* Returns the 1-based index of the first out-of-range param, or 0 if all ok. */ @@ -96,8 +100,8 @@ int check_command_int(const mavlink_command_int_t *cmd) if (r != 0) return r; if (has_location(cmd->command) == 1) { - if (!param_invalid((float)cmd->x, true) && !lat_in_range((float)cmd->x, true)) return 5; - if (!param_invalid((float)cmd->y, true) && !lon_in_range((float)cmd->y, true)) return 6; + if (!coord_invalid_int(cmd->x) && !lat_in_range_int(cmd->x)) return 5; + if (!coord_invalid_int(cmd->y) && !lon_in_range_int(cmd->y)) return 6; } return 0; diff --git a/generator/mavgen_c_cmd_helpers.py b/generator/mavgen_c_cmd_helpers.py index f4f578d1e..d4b447411 100644 --- a/generator/mavgen_c_cmd_helpers.py +++ b/generator/mavgen_c_cmd_helpers.py @@ -113,7 +113,6 @@ def generate(xml_path, output_path): #pragma once #include -#include #include #ifdef __cplusplus @@ -121,22 +120,18 @@ def generate(xml_path, output_path): #endif /** - * @brief Tests if a parameter value is set to an invalid/default value (NaN, possibly 0, or INT32_MAX-derived). - * - * Returns non-zero if param_val is NaN or ±0.0, which are commonly used default values - * for float parameters that are not in use. - * When is_int is true (COMMAND_INT / MISSION_ITEM_INT encoding), also treats values with - * magnitude ≥ 2.0×10⁹ as invalid; INT32_MAX (≈ 2.15×10⁹) is used in p5/p6 to mean - * "use current position". The threshold is set below INT32_MAX but above the maximum - * valid coordinate (±1.8×10⁹). + * @brief Tests if a float command parameter is set to an invalid/default value (NaN or ±0.0). * + * NaN and ±0.0 are commonly used as "not in use" defaults for float parameters. * Define MAV_CMD_STRICT_NAN_INVALID to reject ±0.0 and require NaN only. * + * This is for the float-encoded params (p1–p4, p7, and COMMAND_LONG p5/p6). For the + * int32 (degE7) coordinate fields of COMMAND_INT / MISSION_ITEM_INT, use coord_invalid_int(). + * * @param param_val Parameter value to test. - * @param is_int True for COMMAND_INT / MISSION_ITEM_INT encoding (activates INT32_MAX check). * @return Non-zero if param_val is invalid, zero otherwise. */ -static inline int param_invalid(float param_val, bool is_int) +static inline int param_invalid(float param_val) {{ \tuint32_t bits; \t__builtin_memcpy(&bits, ¶m_val, sizeof(bits)); @@ -144,36 +139,65 @@ def generate(xml_path, output_path): #ifndef MAV_CMD_STRICT_NAN_INVALID \tif (!(bits & 0x7FFFFFFFu)) return 1; /* ±0.0 — invalid by default */ #endif -\tif (is_int && (param_val >= 2.0e9f || param_val <= -2.0e9f)) return 1; /* INT32_MAX — invalid */ \treturn 0; }} /** - * @brief Tests if lat is in range for a latitude. + * @brief Tests if an int32 (degE7) coordinate is set to the "use current position" sentinel. + * + * COMMAND_INT / MISSION_ITEM_INT use INT32_MAX in the x/y (lat/lon) fields to mean + * "use the current position". * - * @param lat Latitude to test. - * @param is_int True for COMMAND_INT / MISSION_ITEM_INT (int32 × 1e7, range ±9×10⁸); - * false for COMMAND_LONG (float degrees, range ±90). + * @param coord Coordinate value (latitude or longitude), int32 degrees × 1e7. + * @return Non-zero if coord is the INT32_MAX sentinel, zero otherwise. + */ +static inline int coord_invalid_int(int32_t coord) +{{ +\treturn coord == INT32_MAX; +}} + +/** + * @brief Tests if lat is a valid latitude, COMMAND_INT / MISSION_ITEM_INT encoding (int32 degE7). + * + * @param lat Latitude, int32 degrees × 1e7 (range ±9×10⁸ for ±90°). + * @return 1 if in range, 0 if out of range. + */ +static inline int lat_in_range_int(int32_t lat) +{{ +\treturn lat >= -900000000 && lat <= 900000000; +}} + +/** + * @brief Tests if lat is a valid latitude, COMMAND_LONG encoding (float degrees). + * + * @param lat Latitude in float degrees (range ±90). * @return 1 if in range, 0 if out of range or NaN. */ -static inline int lat_in_range(float lat, bool is_int) +static inline int lat_in_range_float(float lat) +{{ +\treturn lat >= -90.0f && lat <= 90.0f; +}} + +/** + * @brief Tests if lon is a valid longitude, COMMAND_INT / MISSION_ITEM_INT encoding (int32 degE7). + * + * @param lon Longitude, int32 degrees × 1e7 (range ±1.8×10⁹ for ±180°). + * @return 1 if in range, 0 if out of range. + */ +static inline int lon_in_range_int(int32_t lon) {{ -\treturn is_int ? (lat >= -9e8f && lat <= 9e8f) -\t : (lat >= -90.0f && lat <= 90.0f); +\treturn lon >= -1800000000 && lon <= 1800000000; }} /** - * @brief Tests if lon is in range for a longitude. + * @brief Tests if lon is a valid longitude, COMMAND_LONG encoding (float degrees). * - * @param lon Longitude to test. - * @param is_int True for COMMAND_INT / MISSION_ITEM_INT (int32 × 1e7, range ±1.8×10⁹); - * false for COMMAND_LONG (float degrees, range ±180). + * @param lon Longitude in float degrees (range ±180). * @return 1 if in range, 0 if out of range or NaN. */ -static inline int lon_in_range(float lon, bool is_int) +static inline int lon_in_range_float(float lon) {{ -\treturn is_int ? (lon >= -1.8e9f && lon <= 1.8e9f) -\t : (lon >= -180.0f && lon <= 180.0f); +\treturn lon >= -180.0f && lon <= 180.0f; }} """ @@ -268,7 +292,7 @@ def generate(xml_path, output_path): \t\t i < param_bounds_count && param_bounds[i].cmd == cmd; ++i) {{ \t\t\tconst unsigned pidx = (unsigned)param_bounds[i].param - 1u; \t\t\tconst float param_val = params[pidx]; -\t\t\tif (param_invalid(param_val, false)) {{ continue; }} +\t\t\tif (param_invalid(param_val)) {{ continue; }} \t\t\tif (_bound_is_set(param_bounds[i].lo) && param_val < param_bounds[i].lo) \t\t\t\t{{ return (int)param_bounds[i].param; }} \t\t\tif (_bound_is_set(param_bounds[i].hi) && param_val > param_bounds[i].hi) From 4d98c95eb8a229f676ad02855349a2eceec02dfa Mon Sep 17 00:00:00 2001 From: Julian Oes Date: Wed, 24 Jun 2026 11:11:44 +1200 Subject: [PATCH 08/14] mavgen_c: use standard memcpy instead of __builtin_memcpy __builtin_memcpy is a GCC/Clang extension and is not available on all toolchains (e.g. MSVC), which contradicts the header's self-contained, portable promise. The mavlink fixed headers (protocol.h, mavlink_helpers.h) already use memcpy; follow that convention. --- docs/cmd_range_checking.md | 2 +- generator/mavgen_c_cmd_helpers.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/cmd_range_checking.md b/docs/cmd_range_checking.md index 579d0ee26..ba1f47e0d 100644 --- a/docs/cmd_range_checking.md +++ b/docs/cmd_range_checking.md @@ -20,7 +20,7 @@ python3 generator/mavgen.py \ ``` The `mav_cmd_helpers.h` in this case would be generated to the root of each of the generated dialect folders. -Include the result directly — it is a single self-contained `#pragma once` header with no library dependencies (only `` and ``). +Include the result directly — it is a single self-contained `#pragma once` header with no library dependencies (only ``, ``, and ``). ## API diff --git a/generator/mavgen_c_cmd_helpers.py b/generator/mavgen_c_cmd_helpers.py index d4b447411..32adae2f8 100644 --- a/generator/mavgen_c_cmd_helpers.py +++ b/generator/mavgen_c_cmd_helpers.py @@ -114,6 +114,7 @@ def generate(xml_path, output_path): #include #include +#include #ifdef __cplusplus namespace mav_cmd_helpers {{ @@ -134,7 +135,7 @@ def generate(xml_path, output_path): static inline int param_invalid(float param_val) {{ \tuint32_t bits; -\t__builtin_memcpy(&bits, ¶m_val, sizeof(bits)); +\tmemcpy(&bits, ¶m_val, sizeof(bits)); \tif ((bits & 0x7F800000u) == 0x7F800000u) return 1; /* NaN — always invalid */ #ifndef MAV_CMD_STRICT_NAN_INVALID \tif (!(bits & 0x7FFFFFFFu)) return 1; /* ±0.0 — invalid by default */ @@ -260,7 +261,7 @@ def generate(xml_path, output_path): static inline int _bound_is_set(float bound) {{ \tuint32_t bits; -\t__builtin_memcpy(&bits, &bound, sizeof(bits)); +\tmemcpy(&bits, &bound, sizeof(bits)); \treturn (bits & 0x7F800000u) != 0x7F800000u; }} From 795136ea2fd0549df69d4bc79bfbd50fbd806f57 Mon Sep 17 00:00:00 2001 From: Julian Oes Date: Wed, 24 Jun 2026 11:23:59 +1200 Subject: [PATCH 09/14] mavgen_c: wrap cmd helpers in extern "C" instead of a namespace mav_cmd_helpers.h is a generated dialect header, like mavlink.h and the per-message headers, which guard with '#ifdef __cplusplus / extern "C"'. The previous unconditional 'namespace mav_cmd_helpers' diverged from that convention and broke the documented unqualified API under C++ (callers would have needed mav_cmd_helpers:: qualification). Use extern "C" so the same unqualified calls work in both C and C++. --- generator/mavgen_c_cmd_helpers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/generator/mavgen_c_cmd_helpers.py b/generator/mavgen_c_cmd_helpers.py index 32adae2f8..c318f5331 100644 --- a/generator/mavgen_c_cmd_helpers.py +++ b/generator/mavgen_c_cmd_helpers.py @@ -117,7 +117,7 @@ def generate(xml_path, output_path): #include #ifdef __cplusplus -namespace mav_cmd_helpers {{ +extern "C" {{ #endif /** @@ -306,7 +306,7 @@ def generate(xml_path, output_path): }} #ifdef __cplusplus -}} /* namespace mav_cmd_helpers */ +}} /* extern "C" */ #endif """ From c5d4c7325f388894e11ed1dcafd16eb0330238f8 Mon Sep 17 00:00:00 2001 From: Julian Oes Date: Wed, 24 Jun 2026 11:32:28 +1200 Subject: [PATCH 10/14] mavgen_c: range-check a literal 0 instead of treating it as unset check_range previously skipped any param that was NaN or +-0.0, so a value of 0 bypassed bounds even for params whose XML minimum is greater than 0 (e.g. DO_MOTOR_TEST motor instance >= 1, fence vertex count >= 3). Only NaN/Inf are now treated as 'not set' and skipped; a literal 0 is checked like any other value. This prefers a false positive, which can be relaxed by updating the XML ranges, over letting an out-of-range 0 through. Rename the internal _bound_is_set helper to _is_finite since it is now also used to test param values, not just bounds. --- docs/cmd_range_checking.md | 4 ++-- generator/mavgen_c_cmd_helpers.py | 21 +++++++++++++-------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/docs/cmd_range_checking.md b/docs/cmd_range_checking.md index ba1f47e0d..1da0087c5 100644 --- a/docs/cmd_range_checking.md +++ b/docs/cmd_range_checking.md @@ -39,8 +39,8 @@ Include the result directly — it is a single self-contained `#pragma once` hea ### Using the methods The methods are intended to be used in command handlers and during mission upload to reject MAV_CMD with passed values that are out of range. -The `check_range()` method returns `0` if all the passed params are all in range, have no range, or are the sentinel values - NaN/0/INT32MAX-for-param5or6, and otherwise returns the value of the first out of range param. -The `lat_in_range()` and `lon_in_range()` can further be used to check that lat/lon values aren't passed ranges that are bigger than valid lat/lon values. +The `check_range()` method returns `0` if every bounded param is in range or has no range. NaN/Inf params are treated as "not set" and skipped, but a literal `0` *is* range-checked — so a param whose minimum is greater than `0` will reject `0`. This prefers a false positive (which can be relaxed by updating the XML ranges) over letting a bad value through. Otherwise it returns the 1-based index of the first out-of-range param. +The `lat_in_range_*()` and `lon_in_range_*()` helpers can further be used to check that lat/lon values aren't bigger than valid lat/lon ranges. This code shows how you might check a mission item. The result is either 0, or the param number of the first out of range param. diff --git a/generator/mavgen_c_cmd_helpers.py b/generator/mavgen_c_cmd_helpers.py index c318f5331..986370560 100644 --- a/generator/mavgen_c_cmd_helpers.py +++ b/generator/mavgen_c_cmd_helpers.py @@ -257,21 +257,25 @@ def generate(xml_path, output_path): \treturn f < 0 ? -1 : ((f & (int)CMD_FLAG_IS_DESTINATION) != 0); }} -/* Internal: non-zero if bound is a finite value (not NaN / ±Inf). Bit-manipulation avoids isnan(). */ -static inline int _bound_is_set(float bound) +/* Internal: non-zero if v is a finite value (not NaN / ±Inf). Bit-manipulation avoids isnan(). */ +static inline int _is_finite(float v) {{ \tuint32_t bits; -\tmemcpy(&bits, &bound, sizeof(bits)); +\tmemcpy(&bits, &v, sizeof(bits)); \treturn (bits & 0x7F800000u) != 0x7F800000u; }} /** * @brief Validate MAVLink command parameters against XML-defined bounds. * + * NaN/Inf params are treated as "not set" and skipped. A literal 0 IS range-checked, + * so a param whose XML minimum is greater than 0 will reject 0 (we prefer a false + * positive here — ranges can be relaxed later — over letting a bad value through). + * * @param cmd MAV_CMD command ID. * @param p1,p2,p3,p4,p5,p6,p7 Command parameters 1–7. - * @return 0 if all params are in range or param_invalid values (e.g. NaN); 1–7 (1-based index of - * the first failing parameter) otherwise. + * @return 0 if every bounded param is in range or unset; otherwise 1–7, the 1-based + * index of the first failing parameter. */ static inline int check_range(uint16_t cmd, \tfloat p1, float p2, float p3, float p4, @@ -293,10 +297,11 @@ def generate(xml_path, output_path): \t\t i < param_bounds_count && param_bounds[i].cmd == cmd; ++i) {{ \t\t\tconst unsigned pidx = (unsigned)param_bounds[i].param - 1u; \t\t\tconst float param_val = params[pidx]; -\t\t\tif (param_invalid(param_val)) {{ continue; }} -\t\t\tif (_bound_is_set(param_bounds[i].lo) && param_val < param_bounds[i].lo) +\t\t\t/* Skip NaN/Inf "not set" params; a real 0 IS range-checked below. */ +\t\t\tif (!_is_finite(param_val)) {{ continue; }} +\t\t\tif (_is_finite(param_bounds[i].lo) && param_val < param_bounds[i].lo) \t\t\t\t{{ return (int)param_bounds[i].param; }} -\t\t\tif (_bound_is_set(param_bounds[i].hi) && param_val > param_bounds[i].hi) +\t\t\tif (_is_finite(param_bounds[i].hi) && param_val > param_bounds[i].hi) \t\t\t\t{{ return (int)param_bounds[i].param; }} \t\t}} \t\tbreak; From 09c957fbaff2ceda138cfcceae886d4fd13092b0 Mon Sep 17 00:00:00 2001 From: Julian Oes Date: Wed, 24 Jun 2026 11:33:41 +1200 Subject: [PATCH 11/14] mavgen_c: fix mav_cmd_helpers.h provenance comment The generated header named the wrong generator file (mavgen_cmd_helpers.py) and referenced a 'build system / mavlink submodule' regeneration flow that does not necessarily exist. Name the actual generator and the real source XML basename, and describe regeneration generically via mavgen. --- generator/mavgen_c_cmd_helpers.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/generator/mavgen_c_cmd_helpers.py b/generator/mavgen_c_cmd_helpers.py index 986370560..7cec637eb 100644 --- a/generator/mavgen_c_cmd_helpers.py +++ b/generator/mavgen_c_cmd_helpers.py @@ -101,14 +101,14 @@ def generate(xml_path, output_path): flags.append((cmd, f, short)) with open(output_path, "w") as out: - _write(out, bounds, flags) + _write(out, bounds, flags, os.path.basename(xml_path)) return True _HEADER_TOP = """\ -/* AUTO-GENERATED by mavgen_cmd_helpers.py from {xml} — do not edit. - * Regenerated by the build system when the mavlink submodule or XML changes. +/* AUTO-GENERATED by mavgen (mavgen_c_cmd_helpers.py) from {xml} — do not edit. + * Regenerate with mavgen whenever the MAVLink XML definitions change. */ #pragma once @@ -316,10 +316,8 @@ def generate(xml_path, output_path): """ -def _write(out, bounds, flags): - # We need the xml basename for the comment; use a placeholder resolved at call time. - # Instead, just use a generic note. - out.write(_HEADER_TOP.format(xml="common.xml (via dialect include chain)")) +def _write(out, bounds, flags, basename): + out.write(_HEADER_TOP.format(xml="%s and its includes" % basename)) # --- param_bounds table --- out.write("/* Explicit param bounds from XML minValue / maxValue.\n" From f1d2266ef5db064295097196739e1cea39dfbbef Mon Sep 17 00:00:00 2001 From: Julian Oes Date: Wed, 24 Jun 2026 11:38:54 +1200 Subject: [PATCH 12/14] mavgen_c: exclude marker entries from cmd helper tables MAV_CMD enums contain non-command marker entries: the synthetic MAV_CMD_*_ENUM_END the parser appends, and the *_LAST NOP entries that only mark the upper bound of a command group (hasLocation=false, described as NOPs). None are sendable commands, so drop them from the param_bounds and cmd_flags tables. --- generator/mavgen_c_cmd_helpers.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/generator/mavgen_c_cmd_helpers.py b/generator/mavgen_c_cmd_helpers.py index 7cec637eb..2f026ec1c 100644 --- a/generator/mavgen_c_cmd_helpers.py +++ b/generator/mavgen_c_cmd_helpers.py @@ -71,8 +71,15 @@ def generate(xml_path, output_path): # the whole mavgen run. return False - # Collect all MAV_CMD entries, sorted by cmd value - entries = sorted(mav_cmd.entry, key=lambda e: int(e.value)) + # Collect all MAV_CMD entries, sorted by cmd value, excluding marker entries: + # the synthetic MAV_CMD_ENUM_END (end_marker) and the *_LAST NOP entries that only + # mark the upper bound of a command group. Neither is a real, sendable command, so + # neither belongs in the validation tables. + def _is_marker(e): + return e.end_marker or e.name.endswith("_LAST") + + entries = sorted((e for e in mav_cmd.entry if not _is_marker(e)), + key=lambda e: int(e.value)) # Build param bounds table — only params with at least one bound defined bounds = [] # list of (cmd, param_1based, lo_float, hi_float) From b14e8be846b22f949330f75e9f6e76c33599509d Mon Sep 17 00:00:00 2001 From: Julian Oes Date: Wed, 24 Jun 2026 11:49:46 +1200 Subject: [PATCH 13/14] mavgen_c: guard check_range against out-of-bounds param index check_range indexes params[param-1] using the 1-based param number from the bounds table. Valid MAVLink XML keeps this in 1..7, but a malformed or extended entry with param==0 (unsigned wrap) or param>7 would read past the params[7] stack array. Skip any such entry defensively. --- generator/mavgen_c_cmd_helpers.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/generator/mavgen_c_cmd_helpers.py b/generator/mavgen_c_cmd_helpers.py index 2f026ec1c..8cadcf2ee 100644 --- a/generator/mavgen_c_cmd_helpers.py +++ b/generator/mavgen_c_cmd_helpers.py @@ -303,6 +303,9 @@ def _is_marker(e): \t\tfor (unsigned i = start; \t\t i < param_bounds_count && param_bounds[i].cmd == cmd; ++i) {{ \t\t\tconst unsigned pidx = (unsigned)param_bounds[i].param - 1u; +\t\t\t/* Defensive: params are 1–7 in valid XML, but guard against a malformed +\t\t\t * or extended table entry indexing past params[7] (pidx wraps if param==0). */ +\t\t\tif (pidx >= 7u) {{ continue; }} \t\t\tconst float param_val = params[pidx]; \t\t\t/* Skip NaN/Inf "not set" params; a real 0 IS range-checked below. */ \t\t\tif (!_is_finite(param_val)) {{ continue; }} From 5c625d1a95f9db6e3552d2b1578b3c7235f0e49e Mon Sep 17 00:00:00 2001 From: Julian Oes Date: Wed, 24 Jun 2026 11:54:11 +1200 Subject: [PATCH 14/14] mavgen_c: flag +/-Inf params as out-of-range, skip only NaN check_range previously skipped any non-finite param, so +/-Inf was silently accepted as 'not set' alongside NaN. Treat only NaN as the unset sentinel; +/-Inf is now range-checked and fails against any bound on the violated side, consistent with preferring a false positive over passing a bad value. Replace the _is_finite helper with a NaN-specific _is_nan: 'finite' was the wrong concept since it also rejects Inf. Bounds use NaN as their 'unbounded' sentinel and are never Inf, so the same predicate answers 'is this bound set?' correctly. --- docs/cmd_range_checking.md | 2 +- generator/mavgen_c_cmd_helpers.py | 23 +++++++++++++---------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/docs/cmd_range_checking.md b/docs/cmd_range_checking.md index 1da0087c5..5efcdf16a 100644 --- a/docs/cmd_range_checking.md +++ b/docs/cmd_range_checking.md @@ -39,7 +39,7 @@ Include the result directly — it is a single self-contained `#pragma once` hea ### Using the methods The methods are intended to be used in command handlers and during mission upload to reject MAV_CMD with passed values that are out of range. -The `check_range()` method returns `0` if every bounded param is in range or has no range. NaN/Inf params are treated as "not set" and skipped, but a literal `0` *is* range-checked — so a param whose minimum is greater than `0` will reject `0`. This prefers a false positive (which can be relaxed by updating the XML ranges) over letting a bad value through. Otherwise it returns the 1-based index of the first out-of-range param. +The `check_range()` method returns `0` if every bounded param is in range or has no range. Only NaN params are treated as "not set" and skipped; a literal `0` and ±Inf *are* range-checked — so a param whose minimum is greater than `0` will reject `0`, and an infinite value fails against any bound on the violated side. This prefers a false positive (which can be relaxed by updating the XML ranges) over letting a bad value through. Otherwise it returns the 1-based index of the first out-of-range param. The `lat_in_range_*()` and `lon_in_range_*()` helpers can further be used to check that lat/lon values aren't bigger than valid lat/lon ranges. This code shows how you might check a mission item. diff --git a/generator/mavgen_c_cmd_helpers.py b/generator/mavgen_c_cmd_helpers.py index 8cadcf2ee..aa8270e07 100644 --- a/generator/mavgen_c_cmd_helpers.py +++ b/generator/mavgen_c_cmd_helpers.py @@ -264,20 +264,23 @@ def _is_marker(e): \treturn f < 0 ? -1 : ((f & (int)CMD_FLAG_IS_DESTINATION) != 0); }} -/* Internal: non-zero if v is a finite value (not NaN / ±Inf). Bit-manipulation avoids isnan(). */ -static inline int _is_finite(float v) +/* Internal: non-zero if v is NaN (exponent all ones, non-zero mantissa). Inf is NOT NaN. + * Bit-manipulation avoids isnan(). Bounds use NaN as the "unbounded" sentinel, so this + * also answers "is this bound set?" — bounds are only ever finite or NaN, never Inf. */ +static inline int _is_nan(float v) {{ \tuint32_t bits; \tmemcpy(&bits, &v, sizeof(bits)); -\treturn (bits & 0x7F800000u) != 0x7F800000u; +\treturn (bits & 0x7F800000u) == 0x7F800000u && (bits & 0x007FFFFFu) != 0u; }} /** * @brief Validate MAVLink command parameters against XML-defined bounds. * - * NaN/Inf params are treated as "not set" and skipped. A literal 0 IS range-checked, - * so a param whose XML minimum is greater than 0 will reject 0 (we prefer a false - * positive here — ranges can be relaxed later — over letting a bad value through). + * Only NaN params are treated as "not set" and skipped. A literal 0 IS range-checked, + * so a param whose XML minimum is greater than 0 will reject 0; likewise ±Inf is checked + * and fails against any bound on the violated side. (We prefer a false positive here — + * ranges can be relaxed later — over letting a bad value through.) * * @param cmd MAV_CMD command ID. * @param p1,p2,p3,p4,p5,p6,p7 Command parameters 1–7. @@ -307,11 +310,11 @@ def _is_marker(e): \t\t\t * or extended table entry indexing past params[7] (pidx wraps if param==0). */ \t\t\tif (pidx >= 7u) {{ continue; }} \t\t\tconst float param_val = params[pidx]; -\t\t\t/* Skip NaN/Inf "not set" params; a real 0 IS range-checked below. */ -\t\t\tif (!_is_finite(param_val)) {{ continue; }} -\t\t\tif (_is_finite(param_bounds[i].lo) && param_val < param_bounds[i].lo) +\t\t\t/* Skip only NaN "not set" params; a real 0 and ±Inf ARE range-checked below. */ +\t\t\tif (_is_nan(param_val)) {{ continue; }} +\t\t\tif (!_is_nan(param_bounds[i].lo) && param_val < param_bounds[i].lo) \t\t\t\t{{ return (int)param_bounds[i].param; }} -\t\t\tif (_is_finite(param_bounds[i].hi) && param_val > param_bounds[i].hi) +\t\t\tif (!_is_nan(param_bounds[i].hi) && param_val > param_bounds[i].hi) \t\t\t\t{{ return (int)param_bounds[i].param; }} \t\t}} \t\tbreak;