Refactor peripheral/bus/phy APIs (prep for pico/PIO)

- Refactor PHY to be higher-level with a field focus (controller/peripheral address,
  control byte, etc)
  - Phy layer is now device address aware (ACK's automatically) (Hardens
    previously ~implicit design to only support emulating one AVCLAN
    unit per mockingboard build)
- Redesign `phy_active` => `phy_frame_pending` (to allow
  asynchronous/buffered frame reads on pico) and embed within
  Peripheral::read
- Add `deafen` function: like mute, but also ignore incoming messages (i.e.
  don't receive/process at all). Makes `Bus::is_active` always return
  `false`.
- Add debug bus driving functions

Assisted-by: Claude Code (Opus 5)

src/avclan/avclan.h

@@ -147,6 +147,7 @@ struct Error {
     STARTBIT_MALFORMED,
     STARTBIT_TOO_LONG,
     BAD_STARTBIT,
+    NO_FRAME,
     POOL_EMPTY, // non-bus error
   };
 
@@ -155,6 +156,7 @@ struct Error {
     NAK_MESSAGE_LENGTH,
     NAK_CONTROL,
     NAK_ADDRESS,
+    NAK_TOO_LONG,
     NAK, // generic NAK has max severity
     BUSY,
     MUTED,

src/avclan/bus.cc

@@ -26,7 +26,8 @@
        | 1 │ *Acknowledge*
        *repeat `n` times*
 
-  No acknowledge bits are sent for broadcast frames.
+  For broadcast frames the acknowledge (one) bit is sent, but an ACK response
+  (zero) is not expected.
 */
 
 #include <concepts>
@@ -44,16 +45,12 @@
 namespace {
 using Read = avclan::detail::Error::Read;
 using Send = avclan::detail::Error::Send;
-using Bit = avclan::detail::Bit;
 
-struct trailer_bits_t {};
+// The bus spec has a unit that loses arbitration retry rather than fail: "if
-struct no_parity_t : trailer_bits_t {};   // raw bits (the broadcast bit)
+// the unit loses in arbitration, the frame is automatically reset up twice
-struct with_parity_t : trailer_bits_t {}; // bits + parity (controller address)
+// (three times in total)". Only an attempt that is outbid every time is an
-struct with_ack_t : trailer_bits_t {
+// error worth reporting.
-}; // bits + parity + ACK slot (all other fields)
+constexpr uint8_t SEND_ATTEMPTS = 3;
-inline constexpr no_parity_t no_parity{};
-inline constexpr with_parity_t with_parity{};
-inline constexpr with_ack_t with_ack{};
 } // namespace
 
 namespace avclan {
@@ -72,130 +69,60 @@ public:
   Handle(const Handle &) = delete;
   Handle(Handle &&) = delete;
 
-  // NOLINTNEXTLINE(readability-convert-member-functions-to-static)
+  // Forward to phy API (organized so that hal/phy.h isn't public/visible at the
-  Send sendstartbit() { return phy_send_startbit(); };
+  // C++/library level)
-  // NOLINTNEXTLINE(readability-convert-member-functions-to-static)
+  // NOLINTBEGIN(readability-convert-member-functions-to-static)
-  Read readstartbit() { return phy_read_startbit(); };
+  Read read_header(bool *is_unicast) { return phy_read_header(is_unicast); };
+  Read read_controller_addr(uint16_t *addr) {
+    return phy_read_controller_addr(addr);
+  };
+  Read read_peripheral_addr(uint16_t *addr) {
+    return phy_read_peripheral_addr(addr);
+  };
+  Read read_control(uint8_t *control) { return phy_read_control(control); };
+  Read read_length(uint8_t *length) { return phy_read_length(length); };
+  Read read_data(uint8_t *data) { return phy_read_data(data); };
 
-  template <auto N, std::unsigned_integral T,
+  Send send_header(bool is_unicast) { return phy_send_header(is_unicast); };
-            std::derived_from<trailer_bits_t> Trailer>
+  Send send_controller_addr(uint16_t addr) {
-    requires(sizeof(T) < 3 && N < 16 && !std::same_as<Trailer, with_ack_t>)
+    return phy_send_controller_addr(addr);
-  Send send(T bits, Trailer /*tag*/) {
+  };
-    const Bit parity = sendbits<N>(bits);
+  Send send_peripheral_addr(uint16_t addr, bool expect_ack) {
-
+    return phy_send_peripheral_addr(addr, expect_ack);
-    if constexpr (std::is_same_v<Trailer, with_parity_t>)
+  };
-      sendbits<1>(to_underlying(parity));
+  Send send_control(uint8_t control, bool expect_ack) {
-
+    return phy_send_control(control, expect_ack);
-    return Send{0};
+  };
+  Send send_length(uint8_t length, bool expect_ack) {
+    return phy_send_length(length, expect_ack);
+  };
+  Send send_data(uint8_t data, bool expect_ack) {
+    return phy_send_data(data, expect_ack);
+  };
+  // NOLINTEND(readability-convert-member-functions-to-static)
 };
 
-  template <auto N, std::unsigned_integral T>
+void Bus::init(uint16_t address) {
-    requires(sizeof(T) < 3 && N < 16)
-  Send send(T bits, with_ack_t /*tag*/, bool expect_ack) {
-    send<N>(bits, with_parity);
-
-    if (expect_ack)
-      return read_ACK();
-
-    sendbits<1>((uint8_t)1U); // still need to fill the ack bit slot
-    return Send{0};
-  };
-
-  template <auto N, std::unsigned_integral T,
-            std::derived_from<trailer_bits_t> Trailer>
-    requires(sizeof(T) < 3 && N < 16 && !std::same_as<Trailer, with_ack_t>)
-  Read read(T *bits, Trailer /*tag*/) {
-    const Bit calc_parity = readbits<N>(bits);
-    if constexpr (std::is_same_v<Trailer, with_parity_t>) {
-      uint8_t read_parity;
-      readbits<1>(&read_parity);
-      if (to_underlying(calc_parity) != read_parity)
-        return Read::BAD_PARITY;
-    }
-    return Read{0};
-  };
-
-  template <auto N, std::unsigned_integral T, class F>
-    requires(sizeof(T) < 3 && N < 16)
-  Read read(T *bits, with_ack_t /*tag*/, F &&ack) {
-    if (read<N>(bits, with_parity) == Read::BAD_PARITY)
-      return Read::BAD_PARITY;
-
-    if (ack()) {
-      send_ACK();
-    } else {
-      uint8_t slot;
-      readbits<1>(&slot);
-    }
-
-    return Read{0};
-  };
-  template <auto N, std::unsigned_integral T>
-    requires(sizeof(T) < 3 && N < 16)
-  Read read(T *bits, with_ack_t /*tag*/, bool ack) {
-    return read<N>(bits, with_ack, [=]() { return ack; });
-  }
-
-private:
-  static void send_ACK() { phy_send_ack(); };
-  static Send read_ACK() { return phy_read_ack(); };
-
-  template <auto N, class T> Bit sendbits(T bits);
-  template <auto N, class T> Bit readbits(T *bits);
-
-  template <auto N>
-    requires(N > 1 && N < 8)
-  Bit sendbits(uint8_t bits) {
-    return phy_send_bits_u8(&bits, N);
-  };
-  template <auto N>
-    requires(N <= 16)
-  Bit sendbits(uint16_t bits) {
-    return phy_send_bits_u16(&bits, N);
-  };
-  template <auto N>
-    requires(N < 8)
-  Bit readbits(uint8_t *bits) {
-    return static_cast<Bit>(phy_read_bits_u8(bits, N));
-  };
-  template <auto N>
-    requires(N <= 16)
-  Bit readbits(uint16_t *bits) {
-    return static_cast<Bit>(phy_read_bits_u16(bits, N));
-  };
-};
-
-template <> inline Bit Bus::Handle::sendbits<8>(uint8_t bits) {
-  return phy_send_byte(&bits);
-};
-template <> inline Bit Bus::Handle::sendbits<1>(uint8_t bits) {
-  const Bit bit{static_cast<Bit>(bits & 1U)};
-  phy_send_bit(bit);
-  return bit;
-};
-template <> inline Bit Bus::Handle::readbits<8>(uint8_t *bits) {
-  return phy_read_byte(bits);
-};
-
-void Bus::init() {
   // Idempotent: the single Bus is shared by reference, so every Peripheral's
   // ctor calls init() on it — but the hardware must be brought up exactly once
   // (phy_init is not assumed re-entrant/idempotent).
   if (inited_)
     return;
-  phy_init();
+  phy_init(address);
   muted_ = false;    // phy_init leaves the bus TX unmuted
+  deafened_ = false; // Default to listening
   inited_ = true;
 };
 
 // NOLINTNEXTLINE(readability-convert-member-functions-to-static)
-bool Bus::is_active() const { return phy_active(); };
+bool Bus::is_active() const { return !deafened_ && phy_frame_pending(); };
 void Bus::mute(bool mute) {
   phy_mute(mute);
   muted_ = mute; // Only update muted_ *AFTER* hardware has finished muting
 };
+void Bus::deafen(bool deaf) { deafened_ = deaf; }
 
-auto Bus::read(uint16_t address, Frame::Print print)
+auto Bus::read(Frame::Print print)
     -> expected<std::unique_ptr<Frame>, Error::Read> {
   struct errtype {
     Read type;
@@ -213,69 +140,49 @@ auto Bus::read(uint16_t address, Frame::Print print)
   { // bound handle lifetime
     auto handle = get();
 
-    bool shouldACK = false;
+    err.type = handle.read_header(&in->is_unicast);
-    uint8_t tmp = 0;
-
-    err.type = handle.readstartbit();
     if (err.type != Read{0})
       goto handle_err;
 
-    handle.read<1>(&tmp, no_parity);
+    err.type = handle.read_controller_addr(&in->controller_addr);
-    in->is_unicast = (tmp != 0U);
+    if (err.type != Read{0}) {
-
-    if (auto rerr = handle.read<12>(&in->controller_addr, with_parity);
-        rerr == BAD_PARITY) {
-      err.type = BAD_CONTROLLER_PARITY;
       if (print.verbose)
         err.val = in->controller_addr;
 
       goto handle_err;
     }
 
-    // Using lambda for delayed evaluation of peripheral_addr field
+    err.type = handle.read_peripheral_addr(&in->peripheral_addr);
-    // deref, which will be written by the time the lambda is evaluated
+    if (err.type != Read{0}) {
-    auto should_ack_lambda = [&]() {
-      shouldACK = !is_muted() && (in->peripheral_addr == address);
-      return shouldACK;
-    };
-    if (auto rerr =
-            handle.read<12>(&in->peripheral_addr, with_ack, should_ack_lambda);
-        rerr == BAD_PARITY) {
-      err.type = BAD_PERIPHERAL_PARITY;
       if (print.verbose)
         err.val = in->peripheral_addr;
 
       goto handle_err;
     }
 
-    if (auto rerr = handle.read<4>(&in->control, with_ack, shouldACK);
+    err.type = handle.read_control(&in->control);
-        rerr == BAD_PARITY) {
+    if (err.type != Read{0}) {
-      err.type = BAD_CONTROL_PARITY;
       if (print.verbose)
         err.val = in->control;
 
       goto handle_err;
     }
 
-    if (auto rerr = handle.read<8>(&in->length, with_ack, shouldACK);
+    err.type = handle.read_length(&in->length);
-        rerr == BAD_PARITY) {
+    if (err.type != Read{0}) {
-      err.type = BAD_LENGTH_PARITY;
       if (print.verbose)
         err.val = in->length;
 
       goto handle_err;
-    }
+    } else if (in->length == 0 || in->length > Frame::MAXLENGTH) {
-
-    if (in->length == 0 || in->length > Frame::MAXLENGTH) {
       err.type = BAD_LENGTH_RANGE;
       err.val = in->length;
       goto handle_err;
     }
 
     for (uint8_t i = 0; i < in->length; i++) {
-      if (auto rerr = handle.read<8>(&in->data[i], with_ack, shouldACK);
+      err.type = handle.read_data(&in->data[i]);
-          rerr == BAD_PARITY) {
+      if (err.type != Read{0}) {
-        err.type = BAD_DATA_PARITY;
         if (print.verbose)
           err.val = in->data[i];
 
@@ -305,6 +212,7 @@ auto Bus::read(uint16_t address, Frame::Print print)
       case BAD_LENGTH_PARITY: fputs("reading length", stdout); goto VERBOSE;
       case BAD_LENGTH_RANGE: printf("bad length 0x%02X:", err.val); break;
       case BAD_DATA_PARITY: fputs("reading data", stdout); goto VERBOSE;
+      case NO_FRAME:
       case BAD_PARITY:
         __builtin_unreachable();
       VERBOSE:
@@ -343,47 +251,52 @@ auto Bus::send(const Frame &out, Frame::Print print) -> Send {
     goto handle_err;
   }
 
-  { // bound handle lifetime
+  for (uint8_t attempt = 0; attempt < SEND_ATTEMPTS; attempt++) {
-    auto handle = get();
+    auto handle = get(); // bound handle lifetime
 
-    if (handle.sendstartbit() == BUSY) {
+    err.type = Send{0};
-      // Some other device is already driving the bus
+
-      err.type = BUSY;
+    err.type = handle.send_header(out.is_unicast);
+    if (err.type != Send{0}) // BUSY or LOST_ARBITRATION (broadcast)
+      continue;
+
+    err.type = handle.send_controller_addr(out.controller_addr);
+    if (err.type != Send{0}) // LOST_ARBITRATION: other device has lower address
+      continue;
+
+    err.type = handle.send_peripheral_addr(out.peripheral_addr, out.is_unicast);
+    if (err.type != Send{0})
       goto handle_err;
-    }
 
-    handle.send<1>(static_cast<uint8_t>(out.is_unicast), no_parity);
+    err.type = handle.send_control(out.control, out.is_unicast);
-
+    if (err.type != Send{0})
-    handle.send<12>(out.controller_addr, with_parity);
-
-    if (auto serr =
-            handle.send<12>(out.peripheral_addr, with_ack, out.is_unicast);
-        serr == NAK) {
-      err.type = NAK_ADDRESS;
       goto handle_err;
-    }
 
-    if (auto serr = handle.send<4>(out.control, with_ack, out.is_unicast);
+    err.type = handle.send_length(out.length, out.is_unicast);
-        serr == NAK) {
+    if (err.type != Send{0})
-      err.type = NAK_CONTROL;
       goto handle_err;
-    }
-
-    if (auto serr = handle.send<8>(out.length, with_ack, out.is_unicast);
-        serr == NAK) {
-      err.type = NAK_MESSAGE_LENGTH;
-      goto handle_err;
-    }
 
     for (uint8_t i = 0; i < out.length; i++) {
-      if (auto serr = handle.send<8>(out.data[i], with_ack, out.is_unicast);
+      err.type = handle.send_data(out.data[i], out.is_unicast);
-          serr == NAK) {
+      if (err.type != Send{0}) {
-        err.type = NAK_DATA;
         err.val = i;
         goto handle_err;
       }
     }
-  } // destroy handle
+
+    // A phy that only queued the fields above settles them here
+    uint8_t data_i = 0;
+    err.type = phy_send_done(&data_i);
+    if (err.type != Send{0}) {
+      err.val = data_i;
+      goto handle_err;
+    }
+
+    break; // Sent
+  }
+
+  if (err.type != Send{0})
+    goto handle_err; // Outbid (or busy) on every attempt
 
   // back to read mode
   if (false) { // NOLINT(readability-simplify-boolean-expr)
@@ -396,6 +309,7 @@ auto Bus::send(const Frame &out, Frame::Print print) -> Send {
       case NAK_CONTROL:
       case NAK_MESSAGE_LENGTH:
       case NAK_DATA:
+      case NAK_TOO_LONG:
       case NAK:
         fputs(" NAK: ", stdout);
         switch (err.type) {
@@ -403,6 +317,7 @@ auto Bus::send(const Frame &out, Frame::Print print) -> Send {
           case NAK_CONTROL: fputs("Control", stdout); break;
           case NAK_MESSAGE_LENGTH: fputs("Message length", stdout); break;
           case NAK_DATA: printf(" data[%u]", err.val); break;
+          case NAK_TOO_LONG: fputs("too long", stdout); break;
           case NAK:
           case MUTED:
           case BUSY: __builtin_unreachable();
@@ -420,11 +335,17 @@ auto Bus::send(const Frame &out, Frame::Print print) -> Send {
 
 Bus::Handle Bus::get() { return Handle{*this}; };
 
-#if !defined(NDEBUG) && defined(MEASURE_BUS)
+#if !defined(NDEBUG)
-// Debug bit-timing measurement on the one physical bus; instance-scoped for the
+
-// same reason as is_active().
+void Bus::set_dominant() { phy_set_dominant(); }
+void Bus::set_recessive() { phy_set_recessive(); }
+
+  #ifdef MEASURE_BUS
+// Debug bit-timing measurement on the one physical bus; instance-scoped for
+// the same reason as is_active().
 // NOLINTNEXTLINE(readability-convert-member-functions-to-static)
 void Bus::measure() { phy_measure(); }
   #endif
+#endif
 
 } // namespace avclan

src/avclan/bus.hpp

@@ -69,18 +69,29 @@ public:
   Bus(const Bus &) = delete;
   Bus &operator=(const Bus &) = delete;
 
-  void init();
+  // `address` is our own peripheral address; the phy keeps it and acknowledges
+  // frames addressed to it without further instruction. One address per bus.
+  void init(uint16_t address);
 
+  // True when there is a frame to read and we aren't deafened. Depending on the
+  // target that means the bus has gone dominant or a frame is already buffered.
   bool is_active() const;
+
+  // Prevent the device from being active on the bus
   void mute(bool mute);
   bool is_muted() const { return muted_; };
 
+  // Set the device to be deaf to (ie ignore) bus activity
+  void deafen(bool deaf);
+
 #ifndef NDEBUG
   void measure();
+  detail::Error::Send sendbyte(uint8_t byte, bool ack = false);
+  void set_dominant();
+  void set_recessive();
 #endif
 
-  expected<std::unique_ptr<Frame>, Error::Read> read(uint16_t address,
+  expected<std::unique_ptr<Frame>, Error::Read> read(Frame::Print print);
-                                                     Frame::Print print);
   Error::Send send(const Frame &out, Frame::Print print);
 
 private:
@@ -89,6 +100,7 @@ private:
 
   // Assume mute after default ctor; only viable after init call
   bool muted_ = true;
+  bool deafened_ = false;
   bool inited_ = false;
 };
 

src/avclan/hal/phy.h

@@ -12,16 +12,16 @@
 #ifdef __cplusplus
 using Read = avclan::detail::Error::Read;
 using Send = avclan::detail::Error::Send;
-using Bit = avclan::detail::Bit;
 extern "C" {
 #else
 typedef enum Read Read;
 typedef enum Send Send;
-typedef enum Bit Bit;
 #endif
 
 // One-time bring-up of the bus hardware. Leaves the bus idle and TX unmuted.
-void phy_init(void);
+//
+// `address` is our device address.
+void phy_init(uint16_t address);
 
 // Mute/unmute device TX. "Muted" means transmission is disabled (RX is
 // unchanged/still allowed)
@@ -30,57 +30,77 @@ void phy_mute(bool mute);
 // Non-mutating (e.g. theoretically const qualified/-able)
 bool phy_is_muted(void);
 
-// True when bus is driven/"dominant" (logical 0)
+// True when there is a frame to read. This may reflect current bus state (e.g.
-bool phy_active(void);
+// a frame can be synchronously read from the bus) or indicate that a buffered
+// frame is available to "read".
+bool phy_frame_pending(void);
 
 // Bus-transaction guard: quiesce the other async sources (e.g. interrupts)
 // so that bus read/send timing isn't disturbed. Re-enable relevant async
-// sources with `phy_guard_leave`. May be a no-op on a target where contention
+// sources with `phy_guard_leave`.
-// isn't a concern.
+//  - May be a no-op on a target where contention isn't a concern.
+//  - May acquire a hardware lock to prevent concurrent use
 void phy_guard_enter(void);
 void phy_guard_leave(void);
 
-// Validates an incoming start bit; see avclan::detail::Error::Read.
+/* Per-field frame I/O.
-//  Invariants:
+ *
-//    - Must only be called after positive phy_active() call.
+ * Excluding the header and controller_addr send functions, all other send
-Read phy_read_startbit(void);
+ * functions may have asynchronous implementations (e.g. return before the send
-
+ * has completed on the bus). Success is indicated by a zero value `Read` or
-// Acquire the bus and emit a start bit; may return BUSY
+ * `Send` enum. Non-zero error codes indicate a synchronously completed send
-Send phy_send_startbit(void);
+ * failure. Otherwise, `phy_send_done` must be called to block until all queued
-
+ * send's have completed, and may return the error code for a previous (queued)
-/* Returns 0 (`(Send)0`) if the peripheral sent an ACK bit, otherwise returns
+ * send failure; a success return value indicates that all queued send's have
-NAK. An ACK bit is a cooperative bit, where the sender starts (drives the bus)
+ * finished sending over the bus.
-for the sync period, and allows the receiver to drive the bus (or not) to
+ *
-finish a "1" bit.
+ * The read functions are similarly optionally asynchronous, and may return the
+ * results of buffered reads. When this is the case, phy_read_header returns the
+ * relevant error for the entire frame.
+ *
+ * It is invalid to call any later read or send function after getting an
+ * error (e.g. calling `phy_read_data` after `phy_read_length` errored).
+ *
+ * `expect_ack` indicates whether the recipient should be ACK'ing; false for
+ * broadcast frames which don't have a single recipient.
+ *
  */
-Send phy_read_ack(void);
+Read phy_read_header(bool *is_unicast);
-void phy_send_ack(void);
+Read phy_read_controller_addr(uint16_t *addr);
+Read phy_read_peripheral_addr(uint16_t *addr);
+Read phy_read_control(uint8_t *control);
+Read phy_read_length(uint8_t *length);
+Read phy_read_data(uint8_t *data);
 
-// Per-symbol I/O. The send* helpers return the even parity of the bits sent;
+// Send start and broadcast bits. Always synchronous. Returns success or one of
-// the read* helpers return the even parity of the bits read. The _u8/_u16
+// these error values: MUTED, BUSY, or LOST_ARBITRATION (if another device
-// suffixes name the source-operand width. The function implementations need not
+// overrides our frame with a broadcast).
-// all be separate/independent (e.g. all send functions could be redirect to a
+Send phy_send_header(bool is_unicast);
-// single phy_send_bits_u16, etc).
-// N.B: `len` is the number of bits  to send. The
-// C++ send/readbits templates are the only consumers and use constraints to
-// enforce valid len values, so runtime checks are unnecessary.
 
-// Intended for sending parity bits
+// Send the controller address. Always synchronous. Returns success or
-void phy_send_bit(Bit bit);
+// LOST_ARBITRATION (a device with a lower device is sending a frame).
+Send phy_send_controller_addr(uint16_t addr);
+Send phy_send_peripheral_addr(uint16_t addr, bool expect_ack);
+Send phy_send_control(uint8_t control, bool expect_ack);
+Send phy_send_length(uint8_t length, bool expect_ack);
+Send phy_send_data(uint8_t data, bool expect_ack);
 
-// Variants available to minimize unnecessary work for max runtime efficiency
+// Allows asynchronous ports to block until the phy has finished sending the
-Bit phy_send_bits_u8(const uint8_t *bits, int8_t len);
+// frame. Returns success or the relevant field-specific NAK (e.g. NAK_ADDRESS,
-Bit phy_send_bits_u16(const uint16_t *bits, int8_t len);
+// etc) or CONTENDED_BUS. `data_index` is only written to for NAK_DATA. A fully
-Bit phy_send_byte(const uint8_t *byte);
+// synchronous port should always report success.
+Send phy_send_done(uint8_t *data_index);
 
-Bit phy_read_bits_u8(uint8_t *bits, uint8_t len);
+#ifndef NDEBUG
-Bit phy_read_bits_u16(uint16_t *bits, int8_t len);
-Bit phy_read_byte(uint8_t *byte);
 
-#if !defined(NDEBUG) && defined(MEASURE_BUS)
+void phy_set_dominant(void);
+void phy_set_recessive(void);
+
+  #ifdef MEASURE_BUS
 // Sample and dump bus bit timing over the serial link (REPL `M`).
 void phy_measure(void);
   #endif
+#endif
 
 #ifdef __cplusplus
 }

src/avclan/peripheral.hpp

@@ -40,7 +40,7 @@ public:
                 "update the sentinel value in avclan.h");
 
   Peripheral(Bus &bus, uint16_t address) : bus{bus}, address_{address} {
-    bus.init();
+    bus.init(address);
     (std::get<Devs>(devices_).init(), ...);
   }
 
@@ -59,7 +59,10 @@ public:
 
   expected<std::unique_ptr<Frame>, Error::Read>
   read(Frame::Print print = Frame::Print{}) {
-    return bus.read(address_, print);
+    if (!bus.is_active())
+      return unexpected{detail::Error::Read::NO_FRAME};
+
+    return bus.read(print);
   };
 
   expected<std::unique_ptr<Frame>, detail::SendError>

src/avclan/target/avr-attiny3216/CMakeLists.txt

@@ -139,6 +139,7 @@ target_compile_definitions(jnk0le_uart
     PRIVATE F_CPU=${FREQSEL}
 )
 target_compile_options(jnk0le_uart PUBLIC
+    $<$<CONFIG:Debug>:-Og>
     -ffixed-r2 -ffixed-r3 -ffixed-r4) # reserved ISR scratch (SREG=r4, Z=r2:r3)
 target_link_libraries(avclan PUBLIC jnk0le_uart)
 

src/avclan/target/avr-attiny3216/phy_avr.c

@@ -9,6 +9,7 @@
 #include <stdint.h>
 #include <util/atomic.h>
 
+#include "avclan.h"
 #include "hal/cd_timer.h" // statustimer_enable/disable (guard)
 #include "hal/phy.h"
 #include "media_avr.h" // media_sync_during_mask (guard)
@@ -17,6 +18,9 @@
 // avr-libc.
 #include "timing_avr.h" // IWYU pragma: keep
 
+enum bit_t : uint8_t { bit_zero = 0x00, bit_one = 0x01, bit_start = 0x10 };
+typedef enum bit_t Bit;
+
 // USART0 TX ring indices owned by the jnk0le lib; the guard consults them to
 // decide whether to resume the TX drain (DRE interrupt) on leave.
 extern volatile uint8_t tx0_Head, tx0_Tail;
@@ -80,7 +84,7 @@ bool phy_is_muted() {
 }
 
 // True when the bus is being driven (i.e. not idle/floating).
-bool phy_active() { return (!BUS_IS_IDLE) != 0; }
+bool phy_frame_pending() { return (!BUS_IS_IDLE) != 0; }
 
 // Mute device TX on AVCLAN bus
 void phy_mute(bool mute) {
@@ -114,7 +118,7 @@ static void set_AVC_logic_for(uint8_t val, uint16_t period) {
   while (TCB1.CNT <= period) {};
 }
 
-void phy_send_bit(Bit bit) {
+static void phy_send_bit(Bit bit) {
   uint16_t zero_length, one_length;
   switch (bit) {
     case bit_zero:
@@ -135,7 +139,7 @@ void phy_send_bit(Bit bit) {
   set_AVC_logic_for(1, one_length);
 }
 
-void phy_send_ack() {
+static void phy_send_ack() {
   TCB1.CNT = 0;
 
   // Wait for controller to begin ACK bit
@@ -149,7 +153,7 @@ void phy_send_ack() {
   phy_send_bit(bit_zero);
 }
 
-Send phy_read_ack() {
+static Send phy_read_ack() {
   TCB1.CNT = 0;                              // Double reset of TCB1.CNT: here
   set_AVC_logic_for(0, AVCLAN_BIT1_LOGIC_0); // And here (within)
   AVCLAN_setBusIdle();                       // Stop driving bus
@@ -164,13 +168,13 @@ Send phy_read_ack() {
   // Check/wait in case we get here before peripheral finishes ACK bit
   while (!BUS_IS_IDLE) {
     if (TCB1.CNT > AVCLAN_BIT_LENGTH_MAX)
-      return NAK;
+      return NAK_TOO_LONG;
   }
   return (Send)0;
 }
 
 // Send `len` bits on the AVCLAN bus; returns the even parity
-Bit phy_send_bits_u8(const uint8_t *bits, int8_t len) {
+static Bit phy_send_bits_u8(const uint8_t *bits, int8_t len) {
   uint8_t b = *bits;
   uint8_t parity = 0;
   int8_t len_mod8 = 8;
@@ -189,17 +193,19 @@ Bit phy_send_bits_u8(const uint8_t *bits, int8_t len) {
       b <<= 1;
     }
     len_mod8 = 8;
+    // Avoid under-read for last byte
+    if (len > 0)
       b = *--bits;
   }
   return (parity & 1);
 }
 
 // Send `len` bits on the AVCLAN bus; returns the even parity
-Bit phy_send_bits_u16(const uint16_t *bits, int8_t len) {
+static Bit phy_send_bits_u16(const uint16_t *bits, int8_t len) {
   return phy_send_bits_u8((const uint8_t *)bits + 1, len);
 }
 
-Bit phy_send_byte(const uint8_t *byte) {
+static Bit phy_send_byte(const uint8_t *byte) {
   uint8_t b = *byte;
   uint8_t parity = 0;
 
@@ -238,7 +244,7 @@ ISR(TCB0_INT_vect) {
 }
 
 // Read `len` bits on the AVCLAN bus; returns the even parity
-Bit phy_read_bits_u8(uint8_t *bits, uint8_t len) {
+static Bit phy_read_bits_u8(uint8_t *bits, uint8_t len) {
   uint8_t parity;
   ATOMIC_BLOCK(ATOMIC_RESTORESTATE) {
     READING_BYTE = 0;
@@ -265,7 +271,7 @@ Bit phy_read_bits_u8(uint8_t *bits, uint8_t len) {
 }
 
 // Read `len` bits on the AVCLAN bus; returns the even parity
-Bit phy_read_bits_u16(uint16_t *bits, int8_t len) {
+static Bit phy_read_bits_u16(uint16_t *bits, int8_t len) {
   uint8_t parity = 0;
   if (len > 8) {
     uint8_t over = len - 8;
@@ -278,7 +284,7 @@ Bit phy_read_bits_u16(uint16_t *bits, int8_t len) {
 }
 
 // Read a byte on the AVCLAN bus
-Bit phy_read_byte(uint8_t *byte) {
+static Bit phy_read_byte(uint8_t *byte) {
   uint8_t parity;
   ATOMIC_BLOCK(ATOMIC_RESTORESTATE) {
     READING_BYTE = 0;
@@ -304,7 +310,16 @@ Bit phy_read_byte(uint8_t *byte) {
   return (Bit)(parity & 1);
 }
 
-void phy_init() {
+// Our own peripheral address, and whether the frame being read is addressed to
+// it. hal/phy.h puts the ACK decision in the phy, so the latch that used to
+// live in Bus::read lives here now: set when the peripheral address comes in,
+// and reused by every field after it.
+static uint16_t own_address_;
+static bool acking_;
+
+void phy_init(uint16_t address) {
+  own_address_ = address;
+
   // Set pin 6 and 7 as input
   PORTA.DIRCLR = (PIN6_bm | PIN7_bm);
   // Disable input buffer; recommended when using AC
@@ -339,7 +354,7 @@ void phy_init() {
   phy_mute(false); // unmute AVCLAN bus TX
 }
 
-Read phy_read_startbit() {
+static Read phy_read_startbit() {
   // Following HAL header docs, this function is only called after a driven bus
   // was ~recently detected.
   // Two main (designed) entry flows, depending on timing of pulse end w.r.t.
@@ -430,7 +445,7 @@ Read phy_read_startbit() {
 
 // Acquire the bus and emit a start bit. Returns false if another device is
 // already driving the bus (we can't yet do proper CSMA/CD).
-Send phy_send_startbit() {
+static Send phy_send_startbit() {
   // wait for free line
   TCB1.CNT = 0;
   while (BUS_IS_IDLE) {
@@ -459,6 +474,132 @@ Send phy_send_startbit() {
   return (Send)0;
 }
 
+// Field bits, then the wire's parity bit, checked against what we counted.
+static Read read_parity(Bit calc, Read bad) {
+  uint8_t wire = 0;
+  phy_read_bits_u8(&wire, 1);
+  return ((uint8_t)calc != wire) ? bad : (Read)0;
+}
+
+// An acknowledge slot follows the field whether or not we drive it, so a frame
+// that isn't ours still has to have the slot read away.
+static void ack_slot(void) {
+  if (acking_) {
+    phy_send_ack();
+  } else {
+    uint8_t slot = 0;
+    phy_read_bits_u8(&slot, 1);
+  }
+}
+
+Read phy_read_header(bool *is_unicast) {
+  acking_ = false; // New frame; nothing is addressed to us yet
+
+  const Read err = phy_read_startbit();
+  if (err != (Read)0)
+    return err;
+
+  uint8_t bit = 0;
+  phy_read_bits_u8(&bit, 1); // Broadcast bit: one bare bit, no parity
+  *is_unicast = (bit != 0);
+  return (Read)0;
+}
+
+Read phy_read_controller_addr(uint16_t *addr) {
+  // No acknowledge slot: this field is the arbitration window.
+  return read_parity(phy_read_bits_u16(addr, 12), BAD_CONTROLLER_PARITY);
+}
+
+Read phy_read_peripheral_addr(uint16_t *addr) {
+  const Read err =
+      read_parity(phy_read_bits_u16(addr, 12), BAD_PERIPHERAL_PARITY);
+  if (err != (Read)0)
+    return err;
+
+  // The one field that decides the ack; every field after it reuses the answer.
+  acking_ = !phy_is_muted() && (*addr == own_address_);
+  ack_slot();
+  return (Read)0;
+}
+
+Read phy_read_control(uint8_t *control) {
+  const Read err =
+      read_parity(phy_read_bits_u8(control, 4), BAD_CONTROL_PARITY);
+  if (err != (Read)0)
+    return err;
+  ack_slot();
+  return (Read)0;
+}
+
+Read phy_read_length(uint8_t *length) {
+  const Read err = read_parity(phy_read_byte(length), BAD_LENGTH_PARITY);
+  if (err != (Read)0)
+    return err;
+  ack_slot();
+  return (Read)0;
+}
+
+Read phy_read_data(uint8_t *data) {
+  const Read err = read_parity(phy_read_byte(data), BAD_DATA_PARITY);
+  if (err != (Read)0)
+    return err;
+  ack_slot();
+  return (Read)0;
+}
+
+// `expect_ack` is "a NAK here is worth reporting", not "emit the slot": the
+// slot goes out either way, and a broadcast frame simply has nobody to fill it.
+static Send send_ack_slot(bool expect_ack, Send nak) {
+  if (expect_ack) {
+    const Send err = phy_read_ack();
+    return err == NAK ? nak : err;
+  }
+
+  const uint8_t fill = 1U;
+  phy_send_bits_u8(&fill, 1);
+  return (Send)0;
+}
+
+Send phy_send_header(bool is_unicast) {
+  const Send err = phy_send_startbit();
+  if (err != (Send)0)
+    return err;
+
+  const uint8_t bit = (is_unicast) ? 1U : 0U;
+  phy_send_bits_u8(&bit, 1); // Broadcast bit: one bare bit, no parity
+  return (Send)0;
+}
+
+Send phy_send_controller_addr(uint16_t addr) {
+  // No acknowledge slot: this field is the arbitration window.
+  phy_send_bit(phy_send_bits_u16(&addr, 12));
+  return (Send)0;
+}
+
+Send phy_send_peripheral_addr(uint16_t addr, bool expect_ack) {
+  phy_send_bit(phy_send_bits_u16(&addr, 12));
+  return send_ack_slot(expect_ack, NAK_ADDRESS);
+}
+
+Send phy_send_control(uint8_t control, bool expect_ack) {
+  phy_send_bit(phy_send_bits_u8(&control, 4));
+  return send_ack_slot(expect_ack, NAK_CONTROL);
+}
+
+Send phy_send_length(uint8_t length, bool expect_ack) {
+  phy_send_bit(phy_send_byte(&length));
+  return send_ack_slot(expect_ack, NAK_MESSAGE_LENGTH);
+}
+
+Send phy_send_data(uint8_t data, bool expect_ack) {
+  phy_send_bit(phy_send_byte(&data));
+  return send_ack_slot(expect_ack, NAK_DATA);
+}
+
+// Every field above is on the wire, and has reported its own outcome, by the
+// time it returns: nothing is left to wait out or attribute.
+Send phy_send_done([[maybe_unused]] uint8_t *data_index) { return (Send)0; }
+
 /* Disable non-read related interrupts (USART RX, RTC status tick, mic timer)
    during AVCLAN bus transactions so framing isn't disturbed. TCB0 must remain
    enabled. */
@@ -478,7 +619,12 @@ void phy_guard_leave() {
   }
 }
 
-#if !defined(NDEBUG) && defined(MEASURE_BUS)
+#ifndef NDEBUG
+
+void phy_set_dominant(void) { AVCLAN_setBusDriven(); }
+void phy_set_recessive(void) { AVCLAN_setBusIdle(); }
+
+  #ifdef MEASURE_BUS
     #include <stdio.h> // phy_measure() reporting (debug builds only)
 
     // Only used immediately below
@@ -516,4 +662,6 @@ void phy_measure() {
 
   phy_guard_leave();
 }
+
+  #endif
 #endif

src/sniffer.cc

@@ -74,12 +74,10 @@ int main() {
   print_help();
 
   while (true) {
-    if (peripheral.bus_is_active()) {
     if (auto msg = peripheral.read(Print{.print = printAllFrames,
                                          .binary = printBinary,
                                          .verbose = verbose}))
       incoming.push(std::move(*msg));
-    }
 
     if (const auto *in = incoming.peek()) {
       if (auto resp = peripheral.route(*in)) {
@@ -180,6 +178,16 @@ int main() {
           while (peripheral.device<CDChanger>().media_busy()) {}
           puts("end");
           break;
+        case '+':
+          peripheral.get_bus().deafen(true);
+          peripheral.get_bus().set_dominant();
+          puts("Set bus dominant...");
+          break;
+        case '-':
+          peripheral.get_bus().set_recessive();
+          peripheral.get_bus().deafen(false);
+          puts("Set bus recessive...");
+          break;
   #ifdef MEASURE_BUS
         case 'M': peripheral.get_bus().measure(); break;
   #endif
@@ -324,6 +332,8 @@ void print_help() {
        "s - MIC skip forward\n"
        "b - MIC skip backward\n"
        "M - Measure bit-timing (pulse-widths and periods)\n"
+       "+ - Set bus driven/dominant\n"
+       "- - Set bus idle/recessive\n"
 #endif
        "? - Print this message");
 }