kiss_modem: keep FEM replies ordered and deliverable; drop Wio FEM control

- Wio Tracker L1 1W no longer advertises TX gain control: its setter
  drives LORA_PWR_EN, the shared LDO for the SX1262 and the PA, so
  "gain off" would power down the radio.
- A SetFemState that arrives mid-TX is now applied after TxDone is
  queued, so its reply follows TxDone.
- SetFemState replies (immediate or deferred) are retained until they
  fit in the host output queue instead of being dropped, so a change
  that took effect is always acknowledged.
- GetFemState replies queue behind an outstanding SetFemState reply;
  a further SetFemState returns TxBusy until it clears.

docs/kiss_modem_protocol.md

@@ -279,7 +279,7 @@ FemState response data:
 
 Example: enable RX gain without changing TX gain: apply `0x01`, value `0x01`.
 
-If any bit in the apply mask is not in the board's capability mask (including reserved bits), the modem replies `Unsupported` and changes nothing. On success, SetFemState replies with the FemState response carrying the new state. A request received while a packet is on air is applied as soon as that transmission ends, and the FemState response is sent then; a second request during the same transmission returns `TxBusy`. Capabilities come from the board, so a board with a fixed, board-managed FEM (for example RAK3401) reports none.
+If any bit in the apply mask is not in the board's capability mask (including reserved bits), the modem replies `Unsupported` and changes nothing. On success, SetFemState replies with the FemState response carrying the new state. A SetFemState received while a packet is on air (before its TxDone is queued) is held and applied after TxDone, so its FemState response follows TxDone. While a SetFemState is held or its response is still waiting for output space, GetFemState responses are queued behind it in order, and a further SetFemState returns `TxBusy`. Capabilities come from the board, so a board with a fixed, board-managed FEM (for example RAK3401) reports none.
 
 The setting is not persisted; the board's power-on default applies after reboot.
 

examples/kiss_modem/KissModem.cpp

@@ -27,6 +27,9 @@ KissModem::KissModem(Stream& serial, mesh::LocalIdentity& identity, mesh::RNG& r
   _fem_deferred = false;
   _fem_deferred_apply = 0;
   _fem_deferred_value = 0;
+  _fem_reply_pending = false;
+  _fem_reply_ok = false;
+  _fem_deferred_gets = 0;
   resetOutputQueue();
 }
 
@@ -37,6 +40,8 @@ void KissModem::begin() {
   _has_pending_tx = false;
   _tx_state = TX_IDLE;
   _fem_deferred = false;
+  _fem_reply_pending = false;
+  _fem_deferred_gets = 0;
   _next_agc_reset_ms = millis() + (uint32_t)_agc_reset_interval_sec * 1000;
   resetOutputQueue();
 }
@@ -811,12 +816,16 @@ uint8_t KissModem::femValueMask() const {
   return mask;
 }
 
-void KissModem::writeFemState() {
+bool KissModem::queueFemReply(bool ok, bool mark_busy_error) {
+  if (!ok) {
+    const uint8_t err = HW_ERR_UNSUPPORTED;
+    return queueHardwareFrame(HW_RESP_ERROR, &err, 1, mark_busy_error);
+  }
   uint8_t buf[2] = { femCapabilityMask(), femValueMask() };
-  writeHardwareFrame(HW_RESP(HW_CMD_GET_FEM_STATE), buf, 2);
+  return queueHardwareFrame(HW_RESP(HW_CMD_GET_FEM_STATE), buf, 2, mark_busy_error);
 }
 
-void KissModem::applyFemState(uint8_t apply_mask, uint8_t value_mask) {
+bool KissModem::applyFemState(uint8_t apply_mask, uint8_t value_mask) {
   bool ok = true;
   if (apply_mask & HW_FEM_RX_GAIN) {
     ok &= _board.setLoRaFemLnaEnabled((value_mask & HW_FEM_RX_GAIN) != 0);
@@ -824,17 +833,26 @@ void KissModem::applyFemState(uint8_t apply_mask, uint8_t value_mask) {
   if (apply_mask & HW_FEM_TX_GAIN) {
     ok &= _board.setLoRaFemPaGainEnabled((value_mask & HW_FEM_TX_GAIN) != 0);
   }
-  if (ok) {
-    writeFemState();
-  } else {
-    writeHardwareError(HW_ERR_UNSUPPORTED);
-  }
+  return ok;
 }
 
 void KissModem::processDeferredFem() {
-  if (!_fem_deferred || _tx_state == TX_SENDING) return;
-  _fem_deferred = false;
-  applyFemState(_fem_deferred_apply, _fem_deferred_value);
+  // wait for TxDone to be queued so the host sees the TX result before the FEM change
+  if (_fem_deferred && _tx_state != TX_SENDING && _tx_state != TX_DONE_PENDING) {
+    _fem_reply_ok = applyFemState(_fem_deferred_apply, _fem_deferred_value);
+    _fem_deferred = false;
+    _fem_reply_pending = true;
+  }
+  if (_fem_deferred) return;
+  // replies are retained until queued, and GetFemState replies stay behind the Set reply
+  if (_fem_reply_pending) {
+    if (!queueFemReply(_fem_reply_ok, false)) return;
+    _fem_reply_pending = false;
+  }
+  while (_fem_deferred_gets > 0) {
+    if (!queueFemReply(true, false)) return;
+    _fem_deferred_gets--;
+  }
 }
 
 void KissModem::handleSetFemState(const uint8_t* data, uint16_t len) {
@@ -849,20 +867,30 @@ void KissModem::handleSetFemState(const uint8_t* data, uint16_t len) {
     writeHardwareError(HW_ERR_UNSUPPORTED);
     return;
   }
-  if (_tx_state == TX_SENDING) {
+  if (isFemReplyQueued()) {
+    writeHardwareError(HW_ERR_TX_BUSY);
+    return;
+  }
+  if (_tx_state == TX_SENDING || _tx_state == TX_DONE_PENDING) {
     // board setters drive FEM pins directly; changing them mid-packet would corrupt the TX
-    if (_fem_deferred) {
-      writeHardwareError(HW_ERR_TX_BUSY);
-      return;
-    }
     _fem_deferred = true;
     _fem_deferred_apply = apply_mask;
     _fem_deferred_value = value_mask;
     return;
   }
-  applyFemState(apply_mask, value_mask);
+  _fem_reply_ok = applyFemState(apply_mask, value_mask);
+  _fem_reply_pending = true;
+  processDeferredFem();
 }
 
 void KissModem::handleGetFemState() {
-  writeFemState();
+  if (isFemReplyQueued()) {
+    if (_fem_deferred_gets == UINT8_MAX) {
+      writeHardwareError(HW_ERR_TX_BUSY);
+      return;
+    }
+    _fem_deferred_gets++;
+    return;
+  }
+  queueFemReply(true, true);
 }

examples/kiss_modem/KissModem.h

@@ -163,6 +163,9 @@ class KissModem {
   bool _fem_deferred;
   uint8_t _fem_deferred_apply;
   uint8_t _fem_deferred_value;
+  bool _fem_reply_pending;
+  bool _fem_reply_ok;
+  uint8_t _fem_deferred_gets;
   uint8_t _tx_frame_buf[KISS_TX_FRAME_QUEUE_DEPTH][KISS_MAX_ENCODED_FRAME_SIZE];
   uint16_t _tx_frame_len[KISS_TX_FRAME_QUEUE_DEPTH];
   uint16_t _tx_frame_written[KISS_TX_FRAME_QUEUE_DEPTH];
@@ -192,8 +195,9 @@ class KissModem {
   void maybeResetAgc();
   uint8_t femCapabilityMask() const;
   uint8_t femValueMask() const;
-  void writeFemState();
-  void applyFemState(uint8_t apply_mask, uint8_t value_mask);
+  bool queueFemReply(bool ok, bool mark_busy_error);
+  bool applyFemState(uint8_t apply_mask, uint8_t value_mask);
+  bool isFemReplyQueued() const { return _fem_deferred || _fem_reply_pending || _fem_deferred_gets > 0; }
   void processDeferredFem();
 
   void handleGetIdentity();

test/test_kiss_modem/test_hw_controls.cpp

@@ -13,8 +13,9 @@ public:
     for (uint8_t b : bytes) _rx.push(b);
   }
 
-  int availableForWrite() override { return 4096; }
+  int availableForWrite() override { return blocked ? 0 : 4096; }
   size_t write(const uint8_t* buffer, size_t size) override {
+    if (blocked) return 0;
     _writes.insert(_writes.end(), buffer, buffer + size);
     return size;
   }
@@ -49,6 +50,8 @@ public:
     return frames;
   }
 
+  bool blocked = false;
+
 private:
   std::queue<uint8_t> _rx;
   std::vector<uint8_t> _writes;
@@ -254,7 +257,7 @@ TEST_F(KissHwControlTest, FemShortPayloadIsInvalidLength) {
   EXPECT_EQ(hw1({HW_CMD_SET_FEM_STATE, HW_FEM_RX_GAIN}), (std::vector<uint8_t>{HW_RESP_ERROR, HW_ERR_INVALID_LENGTH}));
 }
 
-TEST_F(KissHwControlTest, FemSetDuringTxIsDeferredUntilTxCompletes) {
+TEST_F(KissHwControlTest, FemSetDuringTxIsDeferredUntilAfterTxDone) {
   board.has_lna = true;
   startTxAndHold();
   serial.takeHardwareFrames();
@@ -264,15 +267,79 @@ TEST_F(KissHwControlTest, FemSetDuringTxIsDeferredUntilTxCompletes) {
   EXPECT_EQ(hw1({HW_CMD_SET_FEM_STATE, HW_FEM_RX_GAIN, 0}), (std::vector<uint8_t>{HW_RESP_ERROR, HW_ERR_TX_BUSY}));
 
   radio.send_complete = true;
+  modem.loop();  // TX done -> TX_DONE_PENDING; FEM still held
+  modem.loop();  // TxDone queued -> idle; FEM applied
+  auto frames = serial.takeHardwareFrames();
+  ASSERT_EQ(frames.size(), 2U);
+  EXPECT_EQ(frames[0], (std::vector<uint8_t>{HW_RESP_TX_DONE, 0x01}));
+  EXPECT_EQ(frames[1], (std::vector<uint8_t>{0x9F, 0x01, 0x01}));
+  EXPECT_TRUE(board.lna);
+  EXPECT_EQ(board.lna_sets, 1);
+}
+
+TEST_F(KissHwControlTest, FemGetDuringDeferredSetIsAnsweredAfterSet) {
+  board.has_lna = true;
+  startTxAndHold();
+  serial.takeHardwareFrames();
+
+  EXPECT_TRUE(hw({HW_CMD_SET_FEM_STATE, HW_FEM_RX_GAIN, HW_FEM_RX_GAIN}).empty());
+  EXPECT_TRUE(hw({HW_CMD_GET_FEM_STATE}).empty());
+
+  radio.send_complete = true;
+  modem.loop();
   modem.loop();
   auto frames = serial.takeHardwareFrames();
-  ASSERT_EQ(frames.size(), 1U);
-  EXPECT_EQ(frames[0], (std::vector<uint8_t>{0x9F, 0x01, 0x01}));
-  EXPECT_TRUE(board.lna);
-  modem.loop();
-  frames = serial.takeHardwareFrames();
-  ASSERT_EQ(frames.size(), 1U);
+  ASSERT_EQ(frames.size(), 3U);
   EXPECT_EQ(frames[0], (std::vector<uint8_t>{HW_RESP_TX_DONE, 0x01}));
+  EXPECT_EQ(frames[1], (std::vector<uint8_t>{0x9F, 0x01, 0x01}));
+  EXPECT_EQ(frames[2], (std::vector<uint8_t>{0x9F, 0x01, 0x01}));
+}
+
+TEST_F(KissHwControlTest, DeferredFemRepliesSurviveHostBackpressure) {
+  board.has_lna = true;
+  hw({HW_CMD_SET_SIGNAL_REPORT, 0});  // one output frame per RX packet
+  startTxAndHold();
+  serial.takeHardwareFrames();
+  EXPECT_TRUE(hw({HW_CMD_SET_FEM_STATE, HW_FEM_RX_GAIN, HW_FEM_RX_GAIN}).empty());
+  EXPECT_TRUE(hw({HW_CMD_GET_FEM_STATE}).empty());
+
+  // with writes blocked: RX packet takes slot 1, TxDone slot 2, so the Set reply cannot be queued
+  serial.blocked = true;
+  static constexpr uint8_t PKT[] = {0x01};
+  modem.onPacketReceived(0, 0, PKT, sizeof(PKT));
+  radio.send_complete = true;
+  for (int i = 0; i < 5; i++) modem.loop();
+  EXPECT_TRUE(board.lna);  // applied once TxDone was queued, reply still held
+
+  serial.blocked = false;
+  for (int i = 0; i < 5; i++) modem.loop();
+  EXPECT_EQ(board.lna_sets, 1);
+
+  auto frames = serial.takeHardwareFrames();
+  ASSERT_EQ(frames.size(), 3U);
+  EXPECT_EQ(frames[0], (std::vector<uint8_t>{HW_RESP_TX_DONE, 0x01}));
+  EXPECT_EQ(frames[1], (std::vector<uint8_t>{0x9F, 0x01, 0x01}));
+  EXPECT_EQ(frames[2], (std::vector<uint8_t>{0x9F, 0x01, 0x01}));
+}
+
+TEST_F(KissHwControlTest, ImmediateFemReplySurvivesHostBackpressure) {
+  board.has_lna = true;
+  hw({HW_CMD_SET_SIGNAL_REPORT, 0});
+
+  serial.blocked = true;
+  static constexpr uint8_t PKT[] = {0x01};
+  modem.onPacketReceived(0, 0, PKT, sizeof(PKT));
+  modem.onPacketReceived(0, 0, PKT, sizeof(PKT));  // output queue now full
+  serial.pushRx({KISS_FEND, KISS_CMD_SETHARDWARE, HW_CMD_SET_FEM_STATE, HW_FEM_RX_GAIN, HW_FEM_RX_GAIN, KISS_FEND});
+  modem.loop();
+  EXPECT_TRUE(board.lna);
+
+  serial.blocked = false;
+  for (int i = 0; i < 5; i++) modem.loop();
+  auto frames = serial.takeHardwareFrames();
+  ASSERT_EQ(frames.size(), 1U);  // FemState, not TxBusy
+  EXPECT_EQ(frames[0], (std::vector<uint8_t>{0x9F, 0x01, 0x01}));
+  EXPECT_EQ(board.lna_sets, 1);
 }
 
 }  // namespace

variants/wio-tracker-l1-1w/WioTrackerL1Board.h

@@ -10,14 +10,14 @@ protected:
   bool _is_pa_enabled = false;
   KeyValueStore* _prefs = NULL;
 
+  bool setLoRaFemPaGainEnabled(bool enable);
+  bool isLoRaFemPaGainEnabled() const { return _is_pa_enabled; }
+
 public:
   WioTrackerL1Board() : NRF52Board("WioTrackerL1 OTA") {}
   void begin();
   void attachDynamicPrefs(KeyValueStore* prefs);
   bool handleCommand(const char* command, uint32_t sender_timestamp, char* reply) override;
-  bool canControlLoRaFemPaGain() const override { return true; }
-  bool setLoRaFemPaGainEnabled(bool enable) override;
-  bool isLoRaFemPaGainEnabled() const override { return _is_pa_enabled; }
 
 #if defined(P_LORA_TX_LED)
   void onBeforeTransmit() override {