From 773ac07a9c8b7da4c10d7c10ff390d6f4c73b228 Mon Sep 17 00:00:00 2001 From: Jens Ogorek Date: Mon, 2 Nov 2026 15:23:36 +0100 Subject: [PATCH 1/3] Return from Can::send() whether the frame was handed to the driver Can::send() drops a frame while the bus is not RUNNING and ignored the result of twai_transmit(), so a module waiting for an answer could not tell a dropped frame from an unanswered one. It now returns false in both cases. Every caller ignores the result so far. --- main/modules/can.cpp | 10 +++++----- main/modules/can.h | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/main/modules/can.cpp b/main/modules/can.cpp index f0a36aa2..9d5fbda8 100644 --- a/main/modules/can.cpp +++ b/main/modules/can.cpp @@ -173,7 +173,7 @@ bool Can::receive() { return true; } -void Can::send(const uint32_t id, const uint8_t data[8], const bool rtr, uint8_t dlc) const { +bool Can::send(const uint32_t id, const uint8_t data[8], const bool rtr, uint8_t dlc) const { if (id > MAX_EXTENDED_ID) { throw std::runtime_error("CAN ID does not fit into 29 bits"); } @@ -185,7 +185,7 @@ void Can::send(const uint32_t id, const uint8_t data[8], const bool rtr, uint8_t echo("CAN send dropped: bus not RUNNING"); last_drop_log = millis(); } - return; + return false; } twai_message_t message; @@ -197,15 +197,15 @@ void Can::send(const uint32_t id, const uint8_t data[8], const bool rtr, uint8_t message.data[i] = data[i]; } - twai_transmit(&message, pdMS_TO_TICKS(0)); + return twai_transmit(&message, pdMS_TO_TICKS(0)) == ESP_OK; } -void Can::send(uint32_t id, +bool Can::send(uint32_t id, uint8_t d0, uint8_t d1, uint8_t d2, uint8_t d3, uint8_t d4, uint8_t d5, uint8_t d6, uint8_t d7, bool rtr) const { uint8_t data[8] = {d0, d1, d2, d3, d4, d5, d6, d7}; - this->send(id, data, rtr); + return this->send(id, data, rtr); } void Can::call(const std::string method_name, const std::vector arguments) { diff --git a/main/modules/can.h b/main/modules/can.h index 825fb9cb..fd684151 100644 --- a/main/modules/can.h +++ b/main/modules/can.h @@ -29,8 +29,8 @@ class Can : public Module { static const std::map get_defaults(); bool receive(); - void send(const uint32_t id, const uint8_t data[8], const bool rtr = false, const uint8_t dlc = 8) const; - void send(const uint32_t id, + bool send(const uint32_t id, const uint8_t data[8], const bool rtr = false, const uint8_t dlc = 8) const; + bool send(const uint32_t id, const uint8_t d0, const uint8_t d1, const uint8_t d2, const uint8_t d3, const uint8_t d4, const uint8_t d5, const uint8_t d6, const uint8_t d7, const bool rtr = false) const; From e8ed213d8d78c2d7bc854270bcda2f1f747378e4 Mon Sep 17 00:00:00 2001 From: Falko Schindler Date: Tue, 3 Nov 2026 17:36:59 +0100 Subject: [PATCH 2/3] Log a frame that twai_transmit() rejects, and say in the header what the result means A frame the driver rejects, e.g. because the TX queue is full, returned false silently, while a frame dropped because the bus is not RUNNING printed the rate-limited "CAN send dropped" echo. Both paths now share one helper, and the second one names the reason: "TX queue full" for ESP_ERR_TIMEOUT, the ESP-IDF error name otherwise. The header says that true means the driver accepted the frame, not that it was acknowledged on the bus, and the definitions carry the const of the declarations. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01WgKVNX7roCP4Wj4t5CrsE9 --- main/modules/can.cpp | 32 +++++++++++++++++++++----------- main/modules/can.h | 2 ++ 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/main/modules/can.cpp b/main/modules/can.cpp index 9d5fbda8..479df603 100644 --- a/main/modules/can.cpp +++ b/main/modules/can.cpp @@ -18,6 +18,15 @@ static uint32_t to_29_bit(const int64_t value, const std::string &what) { return value; } +// Lizard's own modules send in every step, so a stopped bus or a full TX queue would flood the console. +static void log_dropped_frame(const char *reason) { + static unsigned long last_drop_log = 0; + if (millis_since(last_drop_log) > 1000) { + echo("CAN send dropped: %s", reason); + last_drop_log = millis(); + } +} + static Module_ptr create_can(const std::string &name, const std::vector &arguments, MessageHandler) { Module::expect(arguments, 3, integer, integer, integer); const gpio_num_t rx_pin = (gpio_num_t)arguments[0]->evaluate_integer(); @@ -173,18 +182,14 @@ bool Can::receive() { return true; } -bool Can::send(const uint32_t id, const uint8_t data[8], const bool rtr, uint8_t dlc) const { +bool Can::send(const uint32_t id, const uint8_t data[8], const bool rtr, const uint8_t dlc) const { if (id > MAX_EXTENDED_ID) { throw std::runtime_error("CAN ID does not fit into 29 bits"); } twai_status_info_t status_info; if (twai_get_status_info(&status_info) != ESP_OK || status_info.state != TWAI_STATE_RUNNING) { - static unsigned long last_drop_log = 0; - if (millis_since(last_drop_log) > 1000) { - echo("CAN send dropped: bus not RUNNING"); - last_drop_log = millis(); - } + log_dropped_frame("bus not RUNNING"); return false; } @@ -197,13 +202,18 @@ bool Can::send(const uint32_t id, const uint8_t data[8], const bool rtr, uint8_t message.data[i] = data[i]; } - return twai_transmit(&message, pdMS_TO_TICKS(0)) == ESP_OK; + const esp_err_t result = twai_transmit(&message, pdMS_TO_TICKS(0)); + if (result != ESP_OK) { + log_dropped_frame(result == ESP_ERR_TIMEOUT ? "TX queue full" : esp_err_to_name(result)); + return false; + } + return true; } -bool Can::send(uint32_t id, - uint8_t d0, uint8_t d1, uint8_t d2, uint8_t d3, - uint8_t d4, uint8_t d5, uint8_t d6, uint8_t d7, - bool rtr) const { +bool Can::send(const uint32_t id, + const uint8_t d0, const uint8_t d1, const uint8_t d2, const uint8_t d3, + const uint8_t d4, const uint8_t d5, const uint8_t d6, const uint8_t d7, + const bool rtr) const { uint8_t data[8] = {d0, d1, d2, d3, d4, d5, d6, d7}; return this->send(id, data, rtr); } diff --git a/main/modules/can.h b/main/modules/can.h index fd684151..f90c0928 100644 --- a/main/modules/can.h +++ b/main/modules/can.h @@ -29,6 +29,8 @@ class Can : public Module { static const std::map get_defaults(); bool receive(); + // Both overloads return whether the driver accepted the frame, not whether it was acknowledged on the bus. + // false means the frame was dropped, because the bus is not RUNNING or the TX queue is full. bool send(const uint32_t id, const uint8_t data[8], const bool rtr = false, const uint8_t dlc = 8) const; bool send(const uint32_t id, const uint8_t d0, const uint8_t d1, const uint8_t d2, const uint8_t d3, From 4cd0352cb2e64e7b9071ad3c8481aa117507100f Mon Sep 17 00:00:00 2001 From: Falko Schindler Date: Tue, 3 Nov 2026 17:37:16 +0100 Subject: [PATCH 3/3] Return from RmdMotor::send() at once when Can::send() dropped the frame RmdMotor::send() waited up to 3 ms for the motor's answer and retried twice, also when Can::send() had dropped the frame because the bus is not RUNNING or the TX queue is full. With a stopped bus that cost at least 9 ms and three warnings per motor and step, since step() sends 0x9c every time. Now it returns false right away, without waiting or retrying in the same call: the bus state and the queue do not change within microseconds, and the next step() sends again anyway. The three attempts stay for a frame that was queued but not answered within 3 ms. Callers see no new result: after three failed attempts they got false before as well. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01WgKVNX7roCP4Wj4t5CrsE9 --- main/modules/rmd_motor.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/main/modules/rmd_motor.cpp b/main/modules/rmd_motor.cpp index 6fe47f79..74c10da7 100644 --- a/main/modules/rmd_motor.cpp +++ b/main/modules/rmd_motor.cpp @@ -44,7 +44,9 @@ bool RmdMotor::send(const uint8_t d0, const uint8_t d1, const uint8_t d2, const this->last_msg_id = 0; const int max_attempts = 3; for (int i = 0; i < max_attempts; ++i) { - this->can->send(this->motor_id + 0x140, d0, d1, d2, d3, d4, d5, d6, d7); + if (!this->can->send(this->motor_id + 0x140, d0, d1, d2, d3, d4, d5, d6, d7)) { + return false; // the bus is not RUNNING or the TX queue is full; the next step() sends again anyway + } unsigned long int start = micros(); while (this->last_msg_id != d0 && micros_since(start) < timeout_ms * 1000) { this->can->receive();