fix: v2.24.0 — PWM_OUT write semantics + GPIO input-pin masking (BREAKING)

.fusa-reqs.json

@@ -2368,6 +2368,15 @@
       "level": "ASIL-B",
       "asil": "ASIL-B"
     },
+    {
+      "id": "REQ-GPIO-009",
+      "title": "A write to a pin configured as input leaves that pin's value unchanged",
+      "text": "rcp::gpio::apply_gpio_write shall, for every non-Reconfigure write semantics, commit the write combinator's result to GpioState::values only for bit positions where GpioState::directions selects output, and shall leave GpioState::values unchanged at every bit position where directions selects input, regardless of what the request's operand or combinator result specify for that position.",
+      "standard": "iso26262",
+      "level": "ASIL-B",
+      "asil": "ASIL-B",
+      "tc18": "§13.7.4.3 (\"A write request to an input pin is ignored for this input pin\")"
+    },
     {
       "id": "REQ-SPI-001",
       "title": "SPI channel selection via evt[2:0]",
@@ -2563,18 +2572,20 @@
     {
       "id": "REQ-PWM-002",
       "title": "PWM_OUT write semantics: Replace applies directly",
-      "text": "rcp::pwm::PwmOutEndpoint::handle_write shall apply WriteSemantics::Replace by directly assigning `period` and `active_duration` from the request, matching the single write behavior the OPEN Alliance TC18 Remote Control Protocol Specification's PWM_OUT request-handling section describes.",
+      "text": "rcp::pwm::PwmOutEndpoint::handle_write shall apply WriteSemantics::Replace by directly assigning `period` and `active_duration` from the request, matching one of the eight write semantics the OPEN Alliance TC18 Remote Control Protocol Specification's §13.5 Table 30 GPIO/PWM_OUT row defines.",
       "standard": "iso26262",
       "level": "ASIL-B",
-      "asil": "ASIL-B"
+      "asil": "ASIL-B",
+      "tc18": "§13.5 Table 30 (GPIO/PWM_OUT row, evt[2:0]=000b)"
     },
     {
       "id": "REQ-PWM-003",
-      "title": "PWM_OUT write semantics: Or/And/Xor/Add/Subtract rejected as non-combinable",
-      "text": "rcp::pwm::PwmOutEndpoint::handle_write shall return rcp::endpoint::EndpointErrc::non_combinable_write_semantics without modifying its state for WriteSemantics::Or, ::And, ::Xor, ::Add, and ::Subtract, since the specification's PWM_OUT request-handling section defines no bitmask/saturating-write combination against the endpoint's previous period/active values.",
+      "title": "PWM_OUT write semantics: Or/And/Xor/Add/Subtract combine against the previous state, per field",
+      "text": "rcp::pwm::PwmOutEndpoint::handle_write shall apply WriteSemantics::Or/::And/::Xor/::Add/::Subtract independently against `period` and `active_duration`, each within its own uint16_t range (Add/Subtract saturating at 0x0000/0xFFFF rather than wrapping, per §13.5 Table 30's saturation note and independently of GPIO's own 32-bit saturation bound for the same write semantics).",
       "standard": "iso26262",
       "level": "ASIL-B",
-      "asil": "ASIL-B"
+      "asil": "ASIL-B",
+      "tc18": "§13.5 Table 30 (GPIO/PWM_OUT row, evt[2:0]=001b-011b,101b-110b), TC18.txt L3694-3714"
     },
     {
       "id": "REQ-PWM-004",
@@ -2594,11 +2605,12 @@
     },
     {
       "id": "REQ-PWM-006",
-      "title": "PWM_IN mid-pulse trigger signal",
-      "text": "rcp::pwm::PwmInEndpoint::record_measurement shall notify the MidPulse trigger signal on every call, observable via drain() only when MidPulse was armed via triggers().enable().",
+      "title": "PWM_IN rising/falling edge trigger signals (Table 44)",
+      "text": "rcp::pwm::PwmInEndpoint::record_measurement shall notify both the RisingEdge and FallingEdge trigger signals on every call (one full input-capture cycle spans both edges); record_edge shall notify exactly the one signal it is given, without the other, matching TC18 Table 44's two independent trigger outputs rather than an invented single combined signal.",
       "standard": "iso26262",
       "level": "ASIL-B",
-      "asil": "ASIL-B"
+      "asil": "ASIL-B",
+      "tc18": "§13.7.5.1 Table 44 (pwmi trigger outputs)"
     },
     {
       "id": "REQ-PWM-007",

CMakeLists.txt

@@ -1,7 +1,7 @@
 cmake_minimum_required(VERSION 3.21)
 
 project(cpp-rcp
-    VERSION 2.23.0
+    VERSION 2.24.0
     DESCRIPTION "Remote Control Protocol for automotive zonal architecture (C++)"
     HOMEPAGE_URL "https://github.com/SoundMatt/cpp-RCP"
     LANGUAGES CXX

ROADMAP.md

@@ -2334,6 +2334,58 @@ None of these four are wire-format changes to any *other* endpoint type or
 to the shared AVTPDU/ACF framing; `rcp/endpoint.hpp`,
 `rcp/regmap.hpp`, and every other endpoint header are unmodified.
 
+### Post-hoc write-semantics fix pass (v2.24.0, issues #104, #105; BREAKING)
+
+A requirements-extraction pass flagged three items for follow-up: PWM_OUT's
+write semantics, GPIO's input-pin write handling, and CAN's evt/format
+usage. Investigating each against the OPEN Alliance TC18 Remote Control
+Protocol Specification directly:
+
+- `rcp/pwm.hpp` (issue #104): the "post-hoc wire-format fix pass" above
+  (issue #70) had narrowed `PwmOutEndpoint::handle_write` to
+  `WriteSemantics::Replace` only, reasoning that PWM_OUT's own
+  request-handling section (§13.7.5.3) describes no combination against
+  the endpoint's previous state. That reasoning missed that §13.7.5.3
+  doesn't need to redescribe write semantics: §13.5 Table 30 ("EP specific
+  usage of evt-field") is the table that governs evt[2:0] across every
+  endpoint type, and its GPIO/PWM_OUT row explicitly assigns PWM_OUT the
+  *same* eight write semantics as GPIO — including Add/Subtract, whose own
+  worked examples in that row name PWM_out's duty cycle directly. This is
+  a real regression, not a design choice: `rcp/endpoint.hpp`'s own
+  `saturating_add`/`saturating_subtract` were built templated on the
+  caller's unsigned width specifically so GPIO's 32-bit pin mask and
+  PWM_OUT's narrower period/duration fields could share one
+  implementation, per the v2.4.0 roadmap note issue #70's fix
+  contradicted without updating. `handle_write` now applies every
+  semantics via a dedicated 16-bit-width combinator (not
+  `apply_bitmask_write` directly — that function is hardcoded to
+  `uint32_t`, and calling it against a `uint16_t` field then narrowing the
+  result would saturate at the wrong width, silently wrapping instead of
+  clamping at 0xFFFF), independently per field.
+- `rcp/gpio.hpp` (issue #105): TC18 §13.7.4.3 states that a write request
+  to a pin currently configured as an input is ignored for that pin —
+  inputs are driven externally. `apply_gpio_write` never consulted
+  `GpioState::directions` before this fix: every write semantics,
+  including a bare Replace, overwrote every addressed bit regardless of
+  its configured direction, so a request could silently corrupt an input
+  pin's externally-driven value. The write combinator's result is now
+  committed only for bit positions where `directions` selects output,
+  preserving `values` unchanged wherever `directions` selects input.
+- CAN format: investigated and found to have no live code path. This
+  module (`rcp/can.hpp`) is validation-only — identifier range checks,
+  frame-format payload ceilings, XL acceptance filtering — with no
+  ACF-level wire encode/decode for CAN requests/responses anywhere in this
+  codebase. The `.fusa-reqs.json` entries for CAN accurately describe this
+  validation-only scope; there is no evt[2:0]-carries-format defect to fix
+  because there is no wire codec to have it. A full CAN ACF wire codec is
+  a separate, much larger feature gap than a bug fix and is out of scope
+  here — left as a known gap, not silently resolved.
+
+Both real fixes are behavior changes for any caller relying on the
+previous (incorrect) behavior — BREAKING. `rcp/pwm.hpp`/`rcp/gpio.hpp`
+are the only files touched; every other endpoint header, `rcp/endpoint.hpp`
+itself, and the shared AVTPDU/ACF framing are unmodified.
+
 ---
 ### Appendix A — Legacy Roadmap (v0.1.0–v1.11, superseded)
 ---

include/rcp/gpio.hpp

@@ -6,6 +6,7 @@
 // fusa:req REQ-GPIO-006
 // fusa:req REQ-GPIO-007
 // fusa:req REQ-GPIO-008
+// fusa:req REQ-GPIO-009
 
 // GPIO endpoint (ep_type 0x02) — the OPEN Alliance TC18 Remote Control
 // Protocol Specification v0.5.1_RC's simplest endpoint type: a 32-pin
@@ -94,6 +95,17 @@ inline std::error_code make_error_code(GpioErrc e) noexcept {
 // Reconfigure by treating `operand` as the *new pin-direction mask*
 // (replacing, not combining, `state.directions`) rather than a value
 // combined with the pin-level state (extraction §5.3, §4.5 Group C).
+//
+// Input-pin write masking (issue #105, cpp-RCP-15): TC18 §13.7.4.3 states
+// that a write request to a pin currently configured as an input is ignored
+// for that pin — inputs are driven externally, and a write combinator's
+// result for those bit positions must not be committed to state.values.
+// This applied uniformly and without exception before this fix: every write
+// semantics (including a bare Replace) overwrote every addressed bit
+// regardless of state.directions, so a request could silently corrupt an
+// input pin's externally-driven value. The masking below commits the
+// combinator's result only for bits where directions == output (1),
+// preserving state.values unchanged wherever directions == input (0).
 inline std::error_code apply_gpio_write(endpoint::WriteSemantics op, GpioState& state,
                                          PinMask operand) noexcept {
     if (op == endpoint::WriteSemantics::Reconfigure) {
@@ -103,7 +115,7 @@ inline std::error_code apply_gpio_write(endpoint::WriteSemantics op, GpioState&
     uint32_t out = 0;
     auto ec = endpoint::apply_bitmask_write(op, state.values, operand, out);
     if (ec) return ec;
-    state.values = out;
+    state.values = (out & state.directions) | (state.values & ~state.directions);
     return {};
 }
 

include/rcp/pwm.hpp

@@ -34,23 +34,29 @@
 // unchanged — only the two fields' width and the addition of an explicit
 // wire codec are new.
 //
-// PWM_OUT write-semantics fix (issue #70, cpp-RCP-01, second half): this
-// header previously layered rcp/gpio.hpp's/rcp/endpoint.hpp's generic
-// 8-way evt[2:0] bitmask/saturating-write combinator
-// (rcp::endpoint::apply_bitmask_write) onto PWM_OUT's period/active pair.
-// The specification's own PWM_OUT request-handling section describes only
-// one write behavior: a request supplies both values and they take effect
-// directly (a request with PWM_Period of 0 stops generation; the whole
-// point of Add/Or/Xor-style combination — bitwise OR/AND/XOR or saturating
-// add/subtract against the *previous* period/active values — is never
-// described for this endpoint type, and does not obviously make sense for
-// a period/duty-cycle pair the way it does for GPIO's independent pin
-// bits). PwmOutEndpoint::handle_write below therefore no longer calls
-// apply_bitmask_write at all: WriteSemantics::Replace applies the request
-// directly, and every other semantics (Or/And/Xor/Add/Subtract/Reserved/
-// Reconfigure) is rejected via the same
-// EndpointErrc::non_combinable_write_semantics sentinel Reconfigure already
-// used, without reusing apply_bitmask_write's combinator logic itself.
+// PWM_OUT write-semantics correction (issue #104, cpp-RCP-14): issue #70
+// had narrowed PwmOutEndpoint::handle_write to WriteSemantics::Replace only,
+// on the reasoning that the PWM_OUT-specific request-handling section
+// (§13.7.5.3) describes only one write behavior. That reasoning missed that
+// §13.7.5.3 doesn't need to redescribe write semantics at all: §13.5 Table
+// 30 ("EP specific usage of evt-field") is the governing table for
+// evt[2:0]'s meaning across every endpoint type, and its GPIO/PWM_OUT row
+// explicitly assigns PWM_OUT the *same* eight write semantics as GPIO —
+// including Add/Subtract, whose own worked examples in that row name
+// PWM_out's duty cycle directly ("this can be used to increase/decrease the
+// duty cycle of PWM_out"). rcp::endpoint::saturating_add/saturating_subtract
+// (below, in endpoint.hpp) were built templated on the caller's unsigned
+// width specifically so GPIO's 32-bit pin mask and PWM_OUT's narrower
+// period/duration fields could share one implementation — v2.4.0's own
+// roadmap note this file's header used to cite — which issue #70 then
+// contradicted without updating. PwmOutEndpoint::handle_write below now
+// applies every non-Reconfigure write semantics via
+// rcp::endpoint::apply_bitmask_write, per field (period and
+// active_duration independently, each saturating within its own uint16_t
+// range per Table 30's saturation note). Reconfigure remains rejected: this
+// endpoint type has no EP_func addressed-write path implemented yet (a
+// separate, larger gap common to every endpoint but one in this codebase,
+// not specific to PWM_OUT's write-semantics bug this fixes).
 //
 // Field names and behavior below implement TC18's *behavior* as described in
 // an internal structured extraction of the specification named above; no
@@ -137,22 +143,55 @@ inline std::error_code make_error_code(PwmErrc e) noexcept {
     return {static_cast<int>(e), pwm_category()};
 }
 
+// apply_write_field is PWM_OUT's own field-width combinator: it mirrors
+// rcp::endpoint::apply_bitmask_write's case set exactly, but instantiates
+// saturating_add/saturating_subtract at uint16_t rather than uint32_t —
+// apply_bitmask_write itself is hardcoded to uint32_t (see its own
+// declaration in endpoint.hpp), so calling it directly against a uint16_t
+// field and narrowing the uint32_t result back down would compute the
+// Add/Subtract saturation bound at the wrong width (saturating at
+// 0xFFFFFFFF, then truncating — silently wrapping instead of the 0xFFFF
+// saturation Table 30's own note requires for a 16-bit field).
+inline std::error_code apply_write_field(endpoint::WriteSemantics op, uint16_t current,
+                                          uint16_t operand, uint16_t& out) noexcept {
+    switch (op) {
+    case endpoint::WriteSemantics::Replace:  out = operand;                                                 return {};
+    case endpoint::WriteSemantics::Or:       out = static_cast<uint16_t>(current | operand);                return {};
+    case endpoint::WriteSemantics::And:      out = static_cast<uint16_t>(current & operand);                return {};
+    case endpoint::WriteSemantics::Xor:      out = static_cast<uint16_t>(current ^ operand);                return {};
+    case endpoint::WriteSemantics::Add:      out = endpoint::saturating_add<uint16_t>(current, operand);      return {};
+    case endpoint::WriteSemantics::Subtract: out = endpoint::saturating_subtract<uint16_t>(current, operand); return {};
+    case endpoint::WriteSemantics::Reserved:
+        return endpoint::make_error_code(endpoint::EndpointErrc::reserved_write_semantics);
+    case endpoint::WriteSemantics::Reconfigure:
+    default:
+        return endpoint::make_error_code(endpoint::EndpointErrc::non_combinable_write_semantics);
+    }
+}
+
 // ── PwmOutEndpoint (ep_type 0x07) ────────────────────────────────────────────
-// handle_write applies WriteSemantics::Replace directly (the only write
-// behavior the spec's PWM_OUT request-handling section describes) and
-// rejects every other write semantics as non-combinable — see the header
-// comment above for why this no longer reuses
-// rcp::endpoint::apply_bitmask_write's generic bitmask/saturating-write
-// combinator the way rcp/gpio.hpp does.
+// handle_write applies every write semantics Table 30's GPIO/PWM_OUT row
+// defines — Replace/Or/And/Xor/Add/Subtract — via apply_write_field above,
+// independently against `period` and `active_duration` (each its own
+// uint16_t, so each combines and saturates within its own 16-bit range, per
+// Table 30's saturation note). Reconfigure is rejected: PWM_OUT has no
+// EP_func addressed-write path in this codebase yet (see the header comment
+// above), so there is nothing for it to target.
 class PwmOutEndpoint {
 public:
     std::error_code handle_write(endpoint::WriteSemantics op, PwmValue operand,
                                   PwmValue& out_value) noexcept {
-        if (op != endpoint::WriteSemantics::Replace) {
-            return endpoint::make_error_code(endpoint::EndpointErrc::non_combinable_write_semantics);
-        }
-        state_    = operand;
-        out_value = state_;
+        uint16_t new_period = 0;
+        auto ec = apply_write_field(op, state_.period, operand.period, new_period);
+        if (ec) return ec;
+
+        uint16_t new_active = 0;
+        ec = apply_write_field(op, state_.active_duration, operand.active_duration, new_active);
+        if (ec) return ec;
+
+        state_.period          = new_period;
+        state_.active_duration = new_active;
+        out_value               = state_;
         return {};
     }
 

include/rcp/version.hpp

@@ -6,6 +6,6 @@
 namespace rcp {
 
 // Semantic version of the cpp-RCP implementation (matches the latest git tag).
-constexpr std::string_view kVersion = "2.23.0";
+constexpr std::string_view kVersion = "2.24.0";
 
 } // namespace rcp

tests/test_capi.cpp

@@ -257,9 +257,24 @@ TEST_CASE("capi: rcp_send against a real rcp::mock::Server GPIO endpoint", "[cap
     REQUIRE(rcp_ctrl_init(0, rcp::mock::kGpioByteBusId, mock_server_request_fn, &srv,
                           buf, sizeof(buf), &ctrl) == RCP_OK);
 
+    uint8_t out[8] = {};
+
+    // Pin 0 must be configured as output before a write to it can take
+    // effect (REQ-GPIO-009) — every pin defaults to input.
+    uint8_t reconfig_payload[4] = {0x00, 0x00, 0x00, 0x01};
+    rcp_request_t reconfig_req{};
+    reconfig_req.info.byte_bus_id = rcp::mock::kGpioByteBusId;
+    reconfig_req.info.op          = 1; // write
+    reconfig_req.info.evt_op      = 7; // WriteSemantics::Reconfigure
+    reconfig_req.payload           = reconfig_payload;
+    reconfig_req.payload_len        = sizeof(reconfig_payload);
+    rcp_response_t reconfig_resp{};
+    reconfig_resp.payload     = out;
+    reconfig_resp.payload_cap = sizeof(out);
+    REQUIRE(rcp_send(ctrl, &reconfig_req, &reconfig_resp, 0) == RCP_OK);
+
     // Write pins 0x00000001 (big-endian 4-byte GPIO payload), then read back.
     uint8_t write_payload[4] = {0x00, 0x00, 0x00, 0x01};
-    uint8_t out[8] = {};
 
     rcp_request_t req{};
     req.info.byte_bus_id = rcp::mock::kGpioByteBusId;