reviewer remarks

Signed-off-by: Dima Dorezyuk <[email protected]>
EVerest · Feb 9, 2024 · 14d26f4 · 14d26f4
1 parent d6be126
commit 14d26f4
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 12 deletions.
diff --git a/modules/SerialCommHub/main/serial_communication_hubImpl.cpp b/modules/SerialCommHub/main/serial_communication_hubImpl.cpp
@@ -63,9 +63,9 @@ serial_communication_hubImpl::handle_modbus_read_holding_registers(int& target_d
         auto retry_counter = this->num_resends_on_error;
         while (retry_counter > 0) {
 
-            // EVLOG_info << fmt::format("Try {} Call modbus_client->read_holding_register(id {} addr {} len {})",
-            //                           (int)retry_counter, (uint8_t)target_device_id,
-            //                           (uint16_t)first_register_address, (uint16_t)num_registers_to_read);
+            EVLOG_debug << fmt::format("Try {} Call modbus_client->read_holding_register(id {} addr {} len {})",
+                                       (int)retry_counter, (uint8_t)target_device_id, (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, config.max_packet_size);
@@ -99,9 +99,9 @@ serial_communication_hubImpl::handle_modbus_read_input_registers(int& target_dev
         uint8_t retry_counter{this->num_resends_on_error};
         while (retry_counter-- > 0) {
 
-            // EVLOG_info << fmt::format("Try {} Call modbus_client->read_input_register(id {} addr {} len {})",
-            //                           (int)retry_counter, (uint8_t)target_device_id,
-            //                           (uint16_t)first_register_address, (uint16_t)num_registers_to_read);
+            EVLOG_debug << fmt::format("Try {} Call modbus_client->read_input_register(id {} addr {} len {})",
+                                       (int)retry_counter, (uint8_t)target_device_id, (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, config.max_packet_size);

diff --git a/modules/SerialCommHub/manifest.yaml b/modules/SerialCommHub/manifest.yaml
@@ -37,7 +37,10 @@ provides:
         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.
+        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
         minimum: 0
         maximum: 65536

diff --git a/modules/SerialCommHub/tiny_modbus_rtu.cpp b/modules/SerialCommHub/tiny_modbus_rtu.cpp
@@ -307,7 +307,7 @@ int TinyModbusRTU::read_reply(uint8_t* rxbuf, int rxbuf_len) {
             int bytes_read = read(fd, rxbuf + bytes_read_total, rxbuf_len - bytes_read_total);
             if (bytes_read > 0) {
                 bytes_read_total += bytes_read;
-                // EVLOG_info << "RECVD: " << hexdump(rxbuf, bytes_read_total);
+                EVLOG_debug << "RECVD: " << hexdump(rxbuf, bytes_read_total);
             }
         }
     }
@@ -386,7 +386,7 @@ std::vector<uint16_t> TinyModbusRTU::txrx_impl(uint8_t device_address, FunctionC
         // set checksum in the last 2 bytes
         append_checksum(req.get(), req_len);
 
-        // EVLOG_info << "SEND: " << hexdump(req.get(), req_len);
+        EVLOG_debug << "SEND: " << hexdump(req.get(), req_len);
 
         // clear input and output buffer
         tcflush(fd, TCIOFLUSH);

diff --git a/modules/SerialCommHub/tiny_modbus_rtu.hpp b/modules/SerialCommHub/tiny_modbus_rtu.hpp
@@ -57,9 +57,6 @@ class TinyModbusRTU {
     bool open_device(const std::string& device, int baud, bool ignore_echo,
                      const Everest::GpioSettings& rxtx_gpio_settings, const Parity parity,
                      std::chrono::milliseconds initial_timeout, std::chrono::milliseconds within_message_timeout);
-    std::vector<uint16_t> txrx_impl(uint8_t device_address, FunctionCode function, uint16_t first_register_address,
-                                    uint16_t register_quantity, bool wait_for_reply = true,
-                                    std::vector<uint16_t> request = std::vector<uint16_t>());
 
     std::vector<uint16_t> txrx(uint8_t device_address, FunctionCode function, uint16_t first_register_address,
                                uint16_t register_quantity, uint16_t chunk_size, bool wait_for_reply = true,
@@ -69,6 +66,11 @@ class TinyModbusRTU {
     // Serial interface
     int fd{0};
     bool ignore_echo{false};
+
+    std::vector<uint16_t> txrx_impl(uint8_t device_address, FunctionCode function, uint16_t first_register_address,
+                                    uint16_t register_quantity, bool wait_for_reply = true,
+                                    std::vector<uint16_t> request = std::vector<uint16_t>());
+
     int read_reply(uint8_t* rxbuf, int rxbuf_len);
 
     Everest::Gpio rxtx_gpio;