SerialCommHub: support longer transactions, chunking and single regis…

…ter writes (#535) * make the timeout configurable * split long reads into chunks, to not overcome the maximum ModBus message size. * Add single write command Signed-off-by: Dima Dorezyuk <[email protected]> Signed-off-by: Evgeny Petrov <[email protected]> Co-authored-by: Dima Dorezyuk <[email protected]> Co-authored-by: Dima Dorezyuk <[email protected]>
EVerest · Feb 22, 2024 · 415b164 · 415b164
1 parent 5e589da
commit 415b164
Show file tree

Hide file tree

Showing 7 changed files with 225 additions and 53 deletions.
diff --git a/interfaces/serial_communication_hub.yaml b/interfaces/serial_communication_hub.yaml
@@ -72,6 +72,30 @@ cmds:
       description: Status code of the transfer
       type: string
       $ref: /serial_comm_hub_requests#/StatusCodeEnum
+  modbus_write_single_register:
+    description: >-
+      Send a Modbus RTU 'write single register' command via serial interface
+      to the target hardware. (return value: response)
+    arguments:
+      target_device_id:
+        description: ID (1 byte) of the device to send the commands to
+        type: integer
+        minimum: 0
+        maximum: 255
+      register_address:
+        description: Address of the register to write to (16 bit address)
+        type: integer
+        minimum: 0
+        maximum: 65535
+      data:
+        description: Data content to be written to the above selected register
+        type: integer
+        minimum: 0
+        maximum: 65535
+    result:
+      description: Status code of the transfer
+      type: string
+      $ref: /serial_comm_hub_requests#/StatusCodeEnum
   nonstd_write:
     description: >-
       Non standard mode to write registers in read discrete input mode

diff --git a/modules/SerialCommHub/SerialCommHub.hpp b/modules/SerialCommHub/SerialCommHub.hpp
@@ -27,8 +27,8 @@ class SerialCommHub : public Everest::ModuleBase {
     SerialCommHub(const ModuleInfo& info, std::unique_ptr<serial_communication_hubImplBase> p_main, Conf& config) :
         ModuleBase(info), p_main(std::move(p_main)), config(config){};
 
-    const Conf& config;
     const std::unique_ptr<serial_communication_hubImplBase> p_main;
+    const Conf& config;
 
     // ev@1fce4c5e-0ab8-41bb-90f7-14277703d2ac:v1
     // insert your public definitions here

diff --git a/modules/SerialCommHub/main/serial_communication_hubImpl.cpp b/modules/SerialCommHub/main/serial_communication_hubImpl.cpp
@@ -31,14 +31,16 @@ static std::vector<int> vector_to_int(const std::vector<uint16_t>& response) {
 // Implementation
 
 void serial_communication_hubImpl::init() {
+    using namespace std::chrono;
     Everest::GpioSettings rxtx_gpio_settings;
 
     rxtx_gpio_settings.chip_name = config.rxtx_gpio_chip;
     rxtx_gpio_settings.line_number = config.rxtx_gpio_line;
     rxtx_gpio_settings.inverted = config.rxtx_gpio_tx_high;
 
     if (!modbus.open_device(config.serial_port, config.baudrate, config.ignore_echo, rxtx_gpio_settings,
-                            static_cast<tiny_modbus::Parity>(config.parity))) {
+                            static_cast<tiny_modbus::Parity>(config.parity), milliseconds(config.initial_timeout_ms),
+                            milliseconds(config.within_message_timeout_ms))) {
         EVLOG_AND_THROW(Everest::EverestConfigError(fmt::format("Cannot open serial port {}.", config.serial_port)));
     }
 }
@@ -66,7 +68,7 @@ serial_communication_hubImpl::handle_modbus_read_holding_registers(int& target_d
             //                           (uint16_t)first_register_address, (uint16_t)num_registers_to_read);
 
             response = modbus.txrx(target_device_id, tiny_modbus::FunctionCode::READ_MULTIPLE_HOLDING_REGISTERS,
-                                   first_register_address, num_registers_to_read);
+                                   first_register_address, num_registers_to_read, config.max_packet_size);
             if (response.size() > 0) {
                 break;
             }
@@ -102,7 +104,7 @@ serial_communication_hubImpl::handle_modbus_read_input_registers(int& target_dev
             //                           (uint16_t)first_register_address, (uint16_t)num_registers_to_read);
 
             response = modbus.txrx(target_device_id, tiny_modbus::FunctionCode::READ_INPUT_REGISTERS,
-                                   first_register_address, num_registers_to_read);
+                                   first_register_address, num_registers_to_read, config.max_packet_size);
             if (response.size() > 0) {
                 break;
             }
@@ -140,7 +142,7 @@ types::serial_comm_hub_requests::StatusCodeEnum serial_communication_hubImpl::ha
                                        (uint16_t)data.size());
 
             response = modbus.txrx(target_device_id, tiny_modbus::FunctionCode::WRITE_MULTIPLE_HOLDING_REGISTERS,
-                                   first_register_address, data.size(), true, data);
+                                   first_register_address, data.size(), config.max_packet_size, true, data);
             if (response.size() > 0) {
                 break;
             }
@@ -156,6 +158,38 @@ types::serial_comm_hub_requests::StatusCodeEnum serial_communication_hubImpl::ha
     }
 }
 
+types::serial_comm_hub_requests::StatusCodeEnum
+serial_communication_hubImpl::handle_modbus_write_single_register(int& target_device_id, int& register_address,
+                                                                  int& data) {
+    types::serial_comm_hub_requests::Result result;
+    std::vector<uint16_t> response;
+
+    {
+        std::scoped_lock lock(serial_mutex);
+
+        uint8_t retry_counter{this->num_resends_on_error};
+        while (retry_counter-- > 0) {
+
+            EVLOG_debug << fmt::format("Try {} Call modbus_client->write_single_register(id {} addr {} data {})",
+                                       (int)retry_counter, (uint8_t)target_device_id, (uint16_t)register_address,
+                                       (uint16_t)data);
+
+            response = modbus.txrx(target_device_id, tiny_modbus::FunctionCode::WRITE_SINGLE_HOLDING_REGISTER,
+                                   register_address, 1, config.max_packet_size, true, {static_cast<uint16_t>(data)});
+            if (response.size() > 0) {
+                break;
+            }
+        }
+    }
+    EVLOG_debug << fmt::format("Done writing");
+    // process response
+    if (response.size() > 0) {
+        return types::serial_comm_hub_requests::StatusCodeEnum::Success;
+    } else {
+        return types::serial_comm_hub_requests::StatusCodeEnum::Error;
+    }
+}
+
 void serial_communication_hubImpl::handle_nonstd_write(int& target_device_id, int& first_register_address,
                                                        int& num_registers_to_read) {
 }

diff --git a/modules/SerialCommHub/main/serial_communication_hubImpl.hpp b/modules/SerialCommHub/main/serial_communication_hubImpl.hpp
@@ -28,11 +28,13 @@ struct Conf {
     std::string serial_port;
     int baudrate;
     int parity;
-    int rs485_direction_gpio;
     bool ignore_echo;
     std::string rxtx_gpio_chip;
     int rxtx_gpio_line;
     bool rxtx_gpio_tx_high;
+    int max_packet_size;
+    int initial_timeout_ms;
+    int within_message_timeout_ms;
 };
 
 class serial_communication_hubImpl : public serial_communication_hubImplBase {
@@ -57,6 +59,8 @@ class serial_communication_hubImpl : public serial_communication_hubImplBase {
     virtual types::serial_comm_hub_requests::StatusCodeEnum
     handle_modbus_write_multiple_registers(int& target_device_id, int& first_register_address,
                                            types::serial_comm_hub_requests::VectorUint16& data_raw) override;
+    virtual types::serial_comm_hub_requests::StatusCodeEnum
+    handle_modbus_write_single_register(int& target_device_id, int& register_address, int& data) override;
     virtual void handle_nonstd_write(int& target_device_id, int& first_register_address,
                                      int& num_registers_to_read) override;
     virtual types::serial_comm_hub_requests::Result

diff --git a/modules/SerialCommHub/manifest.yaml b/modules/SerialCommHub/manifest.yaml
@@ -36,6 +36,24 @@ provides:
         description: GPIO direction, false means low for TX, true means high for TX
         type: boolean
         default: false
+      max_packet_size:
+        description: >-
+          Maximum size of a packet to read/write in bytes. Payload exceeding the size will be chunked.
+          The APU size according to [wikipedia](https://en.wikipedia.org/wiki/Modbus) is 256 bytes,
+          which is used as default here.
+        type: integer
+        # 7 is a minimum packet size to transfer a response
+        minimum: 7
+        maximum: 65536
+        default: 256
+      initial_timeout_ms:
+        description: Timeout in ms for the first packet.
+        type: integer
+        default: 500
+      within_message_timeout_ms:
+        description: Timeout in ms for subsequent packets.
+        type: integer
+        default: 100
 metadata:
   license: https://opensource.org/licenses/Apache-2.0
   authors: