vault backup: 2025-09-23 09:14:05

Affected files: .obsidian/workspace.json 99 Work/0 OneSec/OneSecNotes/10 Projects/TeensyFlightcontroller/Parameter System Desing Document.md
2025-09-23 09:14:05 +02:00
parent 26ce03c6bc
commit 852a8fa643
2 changed files with 602 additions and 6 deletions
--- a/Projects/TeensyFlightcontroller/Parameter
+++ b/Projects/TeensyFlightcontroller/Parameter
@@ -0,0 +1,596 @@
+---
+title: Parameter System Desing Document
+created_date: 2025-09-23
+updated_date: 2025-09-23
+aliases:
+tags:
+---
+# Parameter System Desing Document
+
+> Teensy 4.0 • Teensyduino/Arduino framework • No RTOS • ETL containers • Chirp protocol for messaging
+
+> **Scope:** Flight Controller (FC) side only, with **name-as-canonical** parameters and **immediate apply** (no staging yet).
+
+> Enums such as TypeId, ParamStatus, ParamOp come from the Chirp side. Include the relevant Chirp headers; do not redefine them here.
+
+---
+
+## 1) Current Rules / Constraints
+
+* **Platform:** Teensy 4.0, Arduino/Teensyduino framework, no RTOS.
+* **Transport:** **Chirp** is the sole protocol for Get/Set requests and responses.
+* **Design principles**
+
+  1. **Name is canonical.** We identify parameters by a unique, human-readable path.
+  2. **Shallow ownership tree.** Each module owns a **single sub-tree**; keep names shallow: `module.param`.
+  3. **Module-facing interface.** Modules expose parameters via light interfaces; the manager never “reaches in” to private variables.
+* **Non-module params:** System-level, logging, build info, flight mode selectors, serial settings, etc. must also fit the same model.
+
+### Current Chirp Message Structure
+```xml
+<message name="ParameterRequest">
+    <enum name="ParamOp" underlying_type="uint8_t">
+        <entry name="Get" value="0" />
+        <entry name="Set" value="1" />
+        <entry name="GetFullList" value="2" />
+    </enum>
+    <field name="timestamp" type="uint64_t" />
+    <field name="source_id" type="uint8_t" />
+    <field name="transaction_id" type="uint16_t" />
+    <field name="target_component" type="uint8_t" />
+    <field name="op" type="ParamOp" />
+    <field name="id" type="uint32_t" />
+    <field name="name" type="uint8_t" size="64" />
+    <field name="name_size" type="uint8_t" />
+    <field name="type" type="TypeId" />
+    <field name="length" type="uint16_t" />
+    <field name="value" type="uint8_t" size="256" />
+</message>
+
+<message name="ParameterResponse">
+    <enum name="ParamStatus" underlying_type="uint8_t">
+        <entry name="Ok" value="0" />
+        <entry name="NotFound" value="1" />
+        <entry name="InvalidType" value="2" />
+        <entry name="InvalidValue" value="3" />
+        <entry name="AccessDenied" value="4" />
+        <entry name="RebootRequired" value="5" />
+        <entry name="InternalError" value="6" />
+    </enum>
+    <field name="timestamp" type="uint64_t" />
+    <field name="source_id" type="uint8_t" />
+    <field name="transaction_id" type="uint16_t" />
+    <field name="status" type="ParamStatus" />
+    <field name="id" type="uint32_t" />
+    <field name="name" type="uint8_t" size="64" />
+    <field name="name_size" type="uint8_t" />
+    <field name="type" type="TypeId" />
+    <field name="length" type="uint16_t" />
+    <field name="value" type="uint8_t" size="256" />
+</message>
+
+```
+
+---
+
+## 2) Naming
+
+* **Hierarchical dot paths: (Either with `group.name` combination, or directly for `name`)**
+  Examples:
+  `attitudectrl.kp_roll`, `attitudectrl.torque_limit`,
+  `servo_left_flap.offset_angle`, `serial_pilot.baud_rate`,
+  `i2c_line1.clock`,
+  `logger.sink_interface`, `system.fw_version`.
+
+* **Uniqueness:** Names are **case-sensitive** and must be unique within the FC.
+
+* **Ownership:** The **root token** (e.g., `attitudectrl`, `logger`, `serial_pilot`) identifies which module/adapter owns the parameter.
+
+---
+
+## 3) Core Components
+
+### 3.1 ParamRegistry (all descriptors)
+
+A single in-memory catalog describing **every configurable parameter** on the FC.
+The registry answers **what** exists (metadata), **how to read** it, and **how to apply** a new value—**without** the manager touching module internals.
+
+#### Descriptor structure (Globally defined across every system in drone)
+
+* `participant` : which device owns this parameter set (here: `"flight_controller"`).
+  *Note:* other devices (autopilot, co-pilot) will have their own registries in their repos.
+
+* `group` : high-level bucket **within the FC**, typically the module or subsystem root (`"attitudectrl"`, `"logger"`, `"system"`, `"serial_copilot"`, `"rc"`, `"imu_leftwing"`).
+
+* `name` : either the **full name of the parameter**, e.g., `"attitudectrl.kp_roll"` , or partial name so that group + name results in the unique name.
+
+* `type` : `TypeId` (shared enum across Chirp, e.g., `Float`, `Uint16`, …).
+
+* `length` : size in bytes for validation and arrays.
+
+* `default_value` : bytes for default initialization at boot.
+
+* `flags` : bitmask. Possible flags:
+
+  * `Writable` — if **unset**, parameter is read-only.
+  * `RebootRequired` — write accepted but **takes effect after reboot**.
+  * `ImmediateApply` — explicitly safe for immediate side-effects.
+
+* **Accessors (delegates):**
+
+  * `get(name) : bytes ` — read current value from the owning module.
+  * `set(name, bytes) : status` — parse/validate module-local rules and update the module.
+
+> The registry owns **only metadata and function delegates**. Actual values remain inside modules.
+
+#### Example C++ sketch
+
+```cpp
+
+struct ParamDescriptor {
+  const char* participant;       // "flight_controller"
+  const char* group;             // "attitudectrl", "logger", ...
+  const char* name;              // "attitudectrl.kp_roll" (canonical, unique)
+  TypeId      type;
+  uint16_t    length;            // sizeof(float) etc.
+  uint8_t     flags;             // bitmask of ParamFlag
+  const uint8_t* default_bytes;  // pointer to default value bytes
+
+  // Delegates to module-owned code:
+  etl::delegate<ParamStatus(uint8_t* out, uint16_t& out_len)> getter;
+  etl::delegate<ParamStatus(const uint8_t* in, uint16_t in_len, bool& reboot_required)> setter;
+};
+
+class ParamRegistry {
+public:
+  // Fixed capacity
+  bool register_param(const ParamDescriptor& d);     // returns false on duplicate name or full
+  const ParamDescriptor* find(const char* name) const; // or find(group, name) pair depending on full name implementation
+
+private:
+  etl::unordered_map<etl::string<64>, ParamDescriptor, NUM_PARAMS> m_by_name; // pre-sized buckets
+};
+```
+**Note**: `TypeId` , `ParamStatus` definitions exist in relevant chirp header files.
+
+
+> **Getter/Setter notes**
+>
+> * `getter` returns the current committed value from the module into `out`.
+> * `setter` validates and updates the module, sets `reboot_required=true` if the new value won’t take effect until reboot.
+
+---
+
+### 3.2 ParameterManager (global singleton)
+
+The single entry point for **Get/Set** operations and the **Chirp** callback endpoint.
+
+**Responsibilities**
+
+* Own the **ParamRegistry**.
+* Expose a **simple API** used by both:
+
+  * Internal code, and
+  * The **Chirp handler** (by registering a callback for `ParameterRequest`).
+* Apply changes unless the descriptor is `RebootRequired`.
+
+**Minimal API sketch**
+
+```cpp
+class ParameterManager {
+public:
+  static ParameterManager& instance();
+
+  void init(ChirpHandler& chirp); // registers ParamRequest callback
+  ParamRegistry& registry();
+
+  // Internal helpers
+  ParamStatus get_by_name(const char* name, uint8_t* out, uint16_t& out_len, TypeId& out_type) const;
+  ParamStatus set_by_name(const char* name, const uint8_t* in, uint16_t in_len, TypeId type, bool& reboot_required);
+
+  // Chirp callback (wired to opcode for ParameterRequest)
+  void chirp_param_request_callback(const chirp::ParameterRequest& req);
+
+private:
+  ParameterManager() = default;
+  ParamRegistry m_reg;
+};
+```
+
+**Key behaviors**
+
+* **Name resolution** via registry.
+* **Immediate apply:** `set_by_name` calls the descriptor’s `setter`. If success:
+  * If `RebootRequired` flag OR setter sets `reboot_required=true` → reply `RebootRequired`.
+  * Else reply `Ok`.
+* **Get** calls the descriptor’s `getter`.
+* **Chirp bridge:** `chirp_param_request_callback` maps Chirp fields to the API above and produces a `ParameterResponse`.
+
+---
+
+## 4) Module Integration via `IFlightModule`
+
+Grouping flight controller modules under a capability interface like `IFlightModule` can be useful:
+
+* Keeps a **clean boundary**: modules **declare** what they own and how to apply changes—no global manager poking internal data.
+* Enables **testability**: I can unit-test a module’s param parsing/validation in PC native tests without wiring the entire FC.
+* Simplifies **registration**: each module lists its parameters and provides delegates once, during init.
+
+**Example interface**
+
+```cpp
+struct IFlightModule {
+  virtual const char* root_namespace() const = 0;  // e.g., "attitudectrl"
+  virtual void enumerate_params(ParamRegistry& reg) = 0; // Optional. modules can expose dedicated getters/setters per param as how it is currently.
+  virtual ~IFlightModule() = default;
+};
+```
+
+**How it’s used**
+
+* On boot, each module calls `enumerate_params(reg)`.
+* Inside `enumerate_params`, the module **registers its parameters** by building `ParamDescriptor`s with **delegates bound to specific setter/getter methods of `this`**.
+
+**Example — Attitude Controller registers `kp_roll`**
+
+```cpp
+class AttitudeController : public IFlightModule {
+public:
+  const char* root_namespace() const override { return "attitudectrl"; }
+
+  void enumerate_params(ParamRegistry& reg) override {
+    static const float DEFAULT_KP_ROLL = 10.0f; // Either defined here or as a member variable
+    static const uint8_t* def = reinterpret_cast<const uint8_t*>(&DEFAULT_KP_ROLL);
+
+    ParamDescriptor d {
+      "flight_controller",
+      "attitudectrl",
+      "kp_roll", // or "attitudectrl.kp_roll"
+      TypeId::Float,
+      (uint16_t)(sizeof(float)),
+      (uint8_t)(ParamFlag::Writable | ParamFlag::ImmediateApply),
+      def,
+      // getter: copy current kp into out
+      etl::delegate<ParamStatus(uint8_t*, uint16_t&)>::create<AttitudeController, &AttitudeController::get_kp_roll>(this),
+      // setter: parse float, validate, assign
+      etl::delegate<ParamStatus(const uint8_t*, uint16_t, bool&)>::create<AttitudeController, &AttitudeController::set_kp_roll>(this)
+    };
+    reg.register_param(d);
+  }
+
+  // Example getter/setter implementations:
+  ParamStatus get_kp_roll(uint8_t* out, uint16_t& out_len) {
+    const uint16_t needed_length = static_cast<uint16_t>(sizeof(float));
+    if (out_len < needed_length) return ParamStatus::InvalidType;
+    memcpy(out, &m_kp_roll, needed_length);
+    out_len = needed_length;
+    return ParamStatus::Ok;
+  }
+
+  ParamStatus set_kp_gains(const uint8_t* in, uint16_t in_len, bool& reboot_required) {
+    const uint16_t needed_length = static_cast<uint16_t>(sizeof(float));
+    if (in_len != needed_length) return ParamStatus::InvalidType;
+    float v;
+    memcpy(&v, in, needed_length);
+    
+    if (v[i] < min || v[i] > max) return ParamStatus::InvalidValue;
+
+    reboot_required = false; // takes effect immediately
+    return ParamStatus::Ok;
+  }
+
+/* rest of the class definition ... */
+
+private:
+  float m_kp_roll = 10.0f;
+  ...
+};
+```
+
+> This pattern can be replicated this pattern for other attitude controller params like `torque_limit`, indi related params etc.
+
+> Alternatively, we can alter the structure of `ParamDescriptor` in the example without redefining the current parameter structure of attitude controller,
+keeping the array structure for kp, kd gains, torque limits etc. Design supports multiple value arrays as single parameter:
+
+```cpp
+  ParamDescriptor d {
+    "flight_controller",
+    "attitudectrl",
+    "kp_gains", // or "attitudectrl.kp_gains"
+    TypeId::Float,
+    (uint16_t)(3 * sizeof(float)),
+    (uint8_t)(ParamFlag::Writable | ParamFlag::ImmediateApply),
+    def,
+    // getter: copy current kp into out
+    etl::delegate<ParamStatus(uint8_t*, uint16_t&)>::create<AttitudeController, &AttitudeController::get_kp_gains>(this),
+    // setter: parse float, validate, assign
+    etl::delegate<ParamStatus(const uint8_t*, uint16_t, bool&)>::create<AttitudeController, &AttitudeController::set_kp_gains>(this)
+  };
+```
+
+where `kp_gains` is defined as:
+```cpp
+private:
+  etl::array<float, 3> m_kp_gains;
+```
+
+---
+
+## 5) Hardware Parameters via `IHardwareParamAdapter`
+
+Some parameters belong to **hardware drivers** (IMUs, serial baud, PWM ranges). I don’t want the ParameterManager to depend on driver APIs, and most drivers are used by multiple flight modules.
+
+**The adapter idea:** a **tiny object** owned by the module that uses the driver (or owned centrally) which exposes parameters for that hardware subtree, using the **same pattern** as a module:
+
+```cpp
+struct IHardwareParamAdapter {
+  virtual const char* root_namespace() const = 0;   // e.g., "serial" or "imu"
+  virtual void enumerate_params(ParamRegistry& reg) = 0;
+  virtual ~IHardwareParamAdapter() = default;
+};
+
+// Example: Serial adapter exposing baud rate
+class SerialParamAdapter : public IHardwareParamAdapter {
+public:
+  const char* root_namespace() const override { return m_root_namespace; }
+
+  void set_root_namespace(char* root_namespace) { m_root_namespace = root_namespace; } // These setters can be needed for multiple instances of serial e.g `serial_pilot` , `serial_copilot`
+
+  void enumerate_params(ParamRegistry& reg) override {
+    static const uint32_t DefaultBaud = 115200;
+    static const uint8_t* def = reinterpret_cast<const uint8_t*>(&DefaultBaud);
+
+    ParamDescriptor baud {
+      "flight_controller",
+      m_root_namespace,
+      "baud_rate",
+      TypeId::Uint32,
+      (uint16_t)sizeof(uint32_t),
+      (uint8_t)(ParamFlag::Writable | ParamFlag::RebootRequired),
+      def,
+      etl::delegate<ParamStatus(uint8_t*, uint16_t&)>::create<SerialParamAdapter, &SerialParamAdapter::get_baud>(this),
+      etl::delegate<ParamStatus(const uint8_t*, uint16_t, bool&)>::create<SerialParamAdapter, &SerialParamAdapter::set_baud>(this)
+    };
+    reg.register_param(baud);
+  }
+
+  ParamStatus get_baud(uint8_t* out, uint16_t& out_len) {
+    if (out_len < sizeof(uint32_t)) return ParamStatus::InvalidType;
+    memcpy(out, &m_baud, sizeof(uint32_t));
+    out_len = sizeof(uint32_t);
+    return ParamStatus::Ok;
+  }
+
+  ParamStatus set_baud(const uint8_t* in, uint16_t in_len, bool& reboot_required) {
+    if (in_len != sizeof(uint32_t)) return ParamStatus::InvalidType;
+    uint32_t v; memcpy(&v, in, sizeof(uint32_t));
+    if (v < 9600 || v > 3000000) return ParamStatus::InvalidValue;
+    m_baud = v;
+    reboot_required = true; // takes effect after re-init
+    return ParamStatus::Ok;
+  }
+
+private:
+  char* m_root_namespace;
+  uint32_t m_baud = 115200;
+  Serial* m_serial;
+};
+```
+
+**Why it’s useful**
+
+* Keeps **hardware side-effects** localized (re-init serial, reconfigure I2C, etc.).
+* Does not require any change to driver code
+* Multiple modules can **share** one adapter’s namespace (`serial.*`, `imu.*`).
+* Allows easy setup for *multiple hardware instances* (`serial_pilot`, `imu_leftwing`)
+* Adapters register like modules; the manager treats them equivalently.
+
+---
+
+## 6) Boot Sequence (step-by-step)
+
+1. **Construct** the global singleton:
+
+   ```cpp
+   auto& paramManager = ParameterManager::instance();
+   ```
+
+2. **Initialize Chirp and register callback:**
+
+   ```cpp
+   paramManager.init(chirp_handler); // internally: chirp_handler.register_callback(OPCODE_PARAM_REQUEST, &ParameterManager::chirp_param_request_callback);
+   ```
+
+3. **Create modules and adapters using factory method** (AttitudeController, Navigator, Logger, SerialParamAdapter, etc.).
+
+4. **Register parameters**:
+
+    Happens when modules are created, most likely in factory class.
+   ```cpp
+   attitudeController.enumerate_params(paramManager.registry());
+   logger.enumerate_params(paramManager.registry());
+   serialAdapter.enumerate_params(paramManager.registry());
+   // ... others
+   ```
+
+5. **Apply defaults** (optional):
+
+   * Iterate all descriptors and call their `setter(default_bytes, length)` to seed modules with defaults **once** at boot.
+
+6. **Ready for runtime**: FC now answers Chirp Get/Set by **name**.
+
+---
+
+## 7) Runtime Update Flow (example)
+
+**Scenario:** ground station sets `attitudectrl.kp_roll = 18.0f`.
+
+1. **Chirp RX:** `ChirpHandler` parses an incoming `ParameterRequest` with:
+
+   * `op = Set`
+   * `name = "attitudectrl.kp_roll"`
+   * `type = Float`
+   * `length = 4`
+   * `value = { bytes of 18.0f }`
+   * `transaction_id` = 42 (example)
+
+2. **Callback dispatch:** `ChirpHandler` calls
+   `ParameterManager::chirp_param_request_callback(req)`.
+
+3. **Manager logic (simplified):**
+
+   ```cpp
+   void ParameterManager::chirp_param_request_callback(const chirp::ParameterRequest& req) {
+     chirp::ParameterResponse resp{};
+     resp.transaction_id = req.transaction_id;
+     resp.timestamp = micros64(); // or timeManager.time() etc.
+
+     if (req.op == ParamOp::Set) {
+       bool reboot_required = false;
+       ParamStatus st = set_by_name((const char*)req.name, req.value, req.length, req.type, reboot_required);
+
+       resp.status = to_wire_status(st, reboot_required);
+       resp.name_size = req.name_size;
+       memcpy(resp.name, req.name, req.name_size);
+       // Echo type/length; optionally include the applied value via a getter or copy directly from request
+       send_parameter_response(resp);
+     }
+     else if (req.op == ParamOp::Get) {
+       uint8_t buf[256]; uint16_t len = sizeof(buf); TypeId t;
+       ParamStatus st = get_by_name((const char*)req.name, buf, len, t);
+
+       resp.status = to_wire_status(st, /*reboot_required*/false);
+       resp.type = t; resp.length = len;
+       memcpy(resp.value, buf, len);
+       send_parameter_response(resp);
+     }
+   }
+   ```
+
+4. **Validation path inside `set_by_name`:**
+
+   * **Lookup**: registry.find(name) → descriptor `d`; if not found → `NotFound`.
+   * **Type/length check**: `req.type == d.type` and `req.length == d.length`; else → `InvalidType`.
+   * **Access check**:
+
+     * If `!(d.flags & Writable)` → `AccessDenied`.
+     * If `is_armed() && (d.flags & FlightLocked)` → `AccessDenied`.
+   * **Apply**:
+
+     * Call `d.setter(req.value, req.length, reboot_required)`.
+     * If setter returns `InvalidValue` (e.g., out of bounds) → reply `InvalidValue`.
+     * If success and `(d.flags & RebootRequired) || reboot_required` → reply `RebootRequired`.
+     * Else → reply `Ok`.
+
+5. **Module effect:**
+
+   * AttitudeController’s `m_kp_roll` is updated to 18.0f and will be used by the next control step.
+
+---
+
+## 8) Validation Layers (where checks live)
+
+1. **Manager-level (generic)**
+
+   * Name exists in registry.
+   * Type/length match the descriptor.
+   * Flags / access policy (Writable, FlightLocked).
+
+2. **Module-level (semantic)**
+
+   * Bounds and relationships specific to the parameter. (Outside of min-max range, must be non-zero etc.)
+
+**Tiny helper for type/length check**
+
+```cpp
+static bool check_wire_matches_desc(TypeId wire_type, uint16_t wire_len, const ParamDescriptor& d) {
+  return (wire_type == d.type) && (wire_len == d.length);
+}
+```
+
+---
+
+## 9) Example: Wiring it Together
+
+**Register the Chirp callback once**
+
+```cpp
+void ParameterManager::init(ChirpHandler& chirp) {
+  chirp.register_callback(chirp::Opcode::ParameterRequest,
+    etl::delegate<void(const chirp::MessageUnpacked&)>::create<ParameterManager, &ParameterManager::on_any_message>(this));
+}
+
+void ParameterManager::on_any_message(const chirp::MessageUnpacked& msg) {
+  // translate msg → ParameterRequest or ignore if not ParamRequest
+  chirp::ParameterRequest req;
+  if (!decode_parameter_request(msg, req)) return;
+  chirp_param_request_callback(req);
+}
+```
+
+**Set/Get APIs**
+
+```cpp
+ParamStatus ParameterManager::set_by_name(const char* name, const uint8_t* in, uint16_t in_len, TypeId type, bool& reboot_req) {
+  reboot_req = false;
+  const ParamDescriptor* d = m_reg.find(name);
+  if (!d) return ParamStatus::NotFound;
+
+  if (!check_wire_matches_desc(type, in_len, *d)) return ParamStatus::InvalidType;
+
+  if (!(d->flags & (uint8_t)ParamFlag::Writable)) return ParamStatus::AccessDenied;
+  if (is_armed() && (d->flags & (uint8_t)ParamFlag::FlightLocked)) return ParamStatus::AccessDenied;
+
+  ParamStatus st = d->setter(in, in_len, reboot_req);
+  return st;
+}
+
+ParamStatus ParameterManager::get_by_name(const char* name, uint8_t* out, uint16_t& out_len, TypeId& out_type) const {
+  const ParamDescriptor* d = m_reg.find(name);
+  if (!d) return ParamStatus::NotFound;
+  out_type = d->type;
+  return d->getter(out, out_len);
+}
+```
+
+---
+
+## 10) Non-Module/System Parameters
+
+Treat these as **their own roots** or grouped under `system.*` / `logger.*` / `build.*`, etc.
+
+* `logger.level` — writable, immediate.
+* `system.flight_mode` — writable, may be `FlightLocked`.
+* `build.git_sha` — read-only (no `Writable` flag).
+* `serial_pilot.baud_rate` — `RebootRequired` (or requires port re-init), adapter-owned.
+
+They register exactly like module parameters (same `ParamDescriptor` pattern).
+
+---
+
+## 11) Future Considerations
+
+* Parameter **Store** and **StagingBuffer** (two-phase commit).
+  → initial version applies immediately; I'd be a good idea to consider staging the changes and applying in a safe state for later versions.
+
+* Full-list enumeration & chunking (GetFullList) for sharing the parameter list with other devices.
+
+* Persistence to Flash/EEPROM
+
+---
+
+## 12) Checklist to Implement
+
+1. Implement **ParamRegistry** with ETL containers.
+2. Create **ParameterManager** singleton; wire to **ChirpHandler**.
+3. Convert 1–2 modules (e.g., `attitudectrl`, `navigator`) to implement **enumerate\_params** and delegate setters/getters.
+4. Add a **SerialParamAdapter** as a reference hardware example.
+5. Bring up **Get/Set by name** end-to-end and test on PC and on Teensy.
+
+---
+
+## TODOs
+
+Add the full list of current parameters in flight controller, to this document.
+
+Try to fit them in currently proposed structure with proper naming, flags.