Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SerialCommHub: several improvements #731

Merged
merged 10 commits into from
Jul 8, 2024
Merged

SerialCommHub: several improvements #731

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
2bc3892
SerialCommHub: make retry count on error configurable
mhei Jun 14, 2024
f08f88d
SerialCommHub: fix fd usage on invalid values
mhei Jun 14, 2024
6a6d62f
SerialCommHub: refactor error handling to prevent log flooding
mhei Jun 14, 2024
153de78
SerialCommHub: improve hexdump of received packet
mhei Jun 17, 2024
0ad7441
SerialCommHub: optimize memory pre-allocation for result
mhei Jun 17, 2024
3fcc110
SerialCommHub: check for even byte count in response
mhei Jun 17, 2024
6960ddc
SerialCommHub: silence warning about unused return value of write
mhei Jun 17, 2024
a909ddf
SerialCommHub: catch system error for failure to write
barsnick Jul 3, 2024
fd2c256
SerialCommHub: fix reading beyond end of buffer
barsnick Jul 5, 2024
538a20f
Merge branch 'main' into cb-serialcommhub-improvements
barsnick Jul 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
164 changes: 62 additions & 102 deletions modules/SerialCommHub/main/serial_communication_hubImpl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@
#include "serial_communication_hubImpl.hpp"

#include <chrono>
#include <cstdint>
#include <date/date.h>
#include <date/tz.h>
#include <fmt/core.h>
#include <mutex>
#include <typeinfo>

namespace module {
Expand Down Expand Up @@ -38,6 +40,8 @@ void serial_communication_hubImpl::init() {
rxtx_gpio_settings.line_number = config.rxtx_gpio_line;
rxtx_gpio_settings.inverted = config.rxtx_gpio_tx_high;

system_error_logged = false;

if (!modbus.open_device(config.serial_port, config.baudrate, config.ignore_echo, rxtx_gpio_settings,
static_cast<tiny_modbus::Parity>(config.parity), config.rtscts,
milliseconds(config.initial_timeout_ms), milliseconds(config.within_message_timeout_ms))) {
Expand All @@ -48,146 +52,102 @@ void serial_communication_hubImpl::init() {
void serial_communication_hubImpl::ready() {
}

// Commands

types::serial_comm_hub_requests::Result
serial_communication_hubImpl::handle_modbus_read_holding_registers(int& target_device_id, int& first_register_address,
int& num_registers_to_read) {

serial_communication_hubImpl::perform_modbus_request(uint8_t device_address, tiny_modbus::FunctionCode function,
uint16_t first_register_address, uint16_t register_quantity,
bool wait_for_reply, std::vector<uint16_t> request) {
std::scoped_lock lock(serial_mutex);
types::serial_comm_hub_requests::Result result;
std::vector<uint16_t> response;

{
std::scoped_lock lock(serial_mutex);

auto retry_counter = this->num_resends_on_error;
while (retry_counter > 0) {

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);
if (response.size() > 0) {
break;
auto retry_counter = config.retries + 1;

while (retry_counter > 0) {
auto current_trial = config.retries + 1 - retry_counter + 1;

EVLOG_debug << fmt::format("Trial {}/{}: calling {}(id {} addr {}({:#06x}) len {})", current_trial,
config.retries + 1, tiny_modbus::FunctionCode_to_string_with_hex(function),
device_address, first_register_address, first_register_address, register_quantity);

try {
response = modbus.txrx(device_address, function, first_register_address, register_quantity,
config.max_packet_size, wait_for_reply, request);
} catch (const tiny_modbus::TinyModbusException& e) {
auto logmsg = fmt::format("Modbus call {} for device id {} addr {}({:#06x}) failed: {}",
tiny_modbus::FunctionCode_to_string_with_hex(function), device_address,
first_register_address, first_register_address, e.what());

if (retry_counter != 1)
EVLOG_debug << logmsg;
else
EVLOG_warning << logmsg;
} catch (const std::logic_error& e) {
EVLOG_warning << "Logic error in Modbus implementation: " << e.what();
} catch (const std::system_error& e) {
// FIXME: report this to the infrastructure, as soon as an error interface for this is available
// Log this only once, as we are convinced this will not go away
if (not system_error_logged) {
EVLOG_error << "System error in accessing Modbus: [" << e.code() << "] " << e.what();
system_error_logged = true;
}
retry_counter--;
}

if (response.size() > 0)
break;

retry_counter--;
}

EVLOG_debug << fmt::format("Process response (size {})", response.size());
// process response
if (response.size() > 0) {
EVLOG_debug << fmt::format("Process response (size {})", response.size());
result.status_code = types::serial_comm_hub_requests::StatusCodeEnum::Success;
result.value = vector_to_int(response);
system_error_logged = false; // reset after success
} else {
result.status_code = types::serial_comm_hub_requests::StatusCodeEnum::Error;
}
return result;
}

types::serial_comm_hub_requests::Result
serial_communication_hubImpl::handle_modbus_read_input_registers(int& target_device_id, int& first_register_address,
int& num_registers_to_read) {
types::serial_comm_hub_requests::Result result;
std::vector<uint16_t> response;

{
std::scoped_lock lock(serial_mutex);
// Commands

uint8_t retry_counter{this->num_resends_on_error};
while (retry_counter-- > 0) {
types::serial_comm_hub_requests::Result
serial_communication_hubImpl::handle_modbus_read_holding_registers(int& target_device_id, int& first_register_address,
int& 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);
return perform_modbus_request(target_device_id, tiny_modbus::FunctionCode::READ_MULTIPLE_HOLDING_REGISTERS,
first_register_address, 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);
if (response.size() > 0) {
break;
}
}
}
types::serial_comm_hub_requests::Result
serial_communication_hubImpl::handle_modbus_read_input_registers(int& target_device_id, int& first_register_address,
int& num_registers_to_read) {

EVLOG_debug << fmt::format("Process response (size {})", response.size());
// process response
if (response.size() > 0) {
result.status_code = types::serial_comm_hub_requests::StatusCodeEnum::Success;
result.value = vector_to_int(response);
} else {
result.status_code = types::serial_comm_hub_requests::StatusCodeEnum::Error;
}
return result;
return perform_modbus_request(target_device_id, tiny_modbus::FunctionCode::READ_INPUT_REGISTERS,
first_register_address, num_registers_to_read);
}

types::serial_comm_hub_requests::StatusCodeEnum serial_communication_hubImpl::handle_modbus_write_multiple_registers(
int& target_device_id, int& first_register_address, types::serial_comm_hub_requests::VectorUint16& data_raw) {

types::serial_comm_hub_requests::Result result;
std::vector<uint16_t> response;

std::vector<uint16_t> data;
append_array<uint16_t, int>(data, data_raw.data);

{
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_multiple_registers(id {} addr {} len {})",
(int)retry_counter, (uint8_t)target_device_id, (uint16_t)first_register_address,
(uint16_t)data.size());

response = modbus.txrx(target_device_id, tiny_modbus::FunctionCode::WRITE_MULTIPLE_HOLDING_REGISTERS,
first_register_address, data.size(), config.max_packet_size, true, data);
if (response.size() > 0) {
break;
}
}
}
result = perform_modbus_request(target_device_id, tiny_modbus::FunctionCode::WRITE_MULTIPLE_HOLDING_REGISTERS,
first_register_address, data.size(), true, data);

EVLOG_debug << fmt::format("Done writing (size {})", response.size());
// process response
if (response.size() > 0) {
return types::serial_comm_hub_requests::StatusCodeEnum::Success;
} else {
return types::serial_comm_hub_requests::StatusCodeEnum::Error;
}
return result.status_code;
}

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) {
result = perform_modbus_request(target_device_id, tiny_modbus::FunctionCode::WRITE_SINGLE_HOLDING_REGISTER,
register_address, 1, true, {static_cast<uint16_t>(data)});

EVLOG_debug << fmt::format("Try {} Call modbus_client->write_single_register(id {} addr {} data 0x{:04x})",
(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 (size {})", response.size());
// process response
if (response.size() > 0) {
return types::serial_comm_hub_requests::StatusCodeEnum::Success;
} else {
return types::serial_comm_hub_requests::StatusCodeEnum::Error;
}
return result.status_code;
}

void serial_communication_hubImpl::handle_nonstd_write(int& target_device_id, int& first_register_address,
Expand Down
9 changes: 8 additions & 1 deletion modules/SerialCommHub/main/serial_communication_hubImpl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
// insert your custom include headers here
#include "tiny_modbus_rtu.hpp"
#include <chrono>
#include <cstdint>
#include <termios.h>
#include <utils/thread.hpp>
#include <vector>
Expand All @@ -36,6 +37,7 @@
int max_packet_size;
int initial_timeout_ms;
int within_message_timeout_ms;
int retries;

Check notice on line 40 in modules/SerialCommHub/main/serial_communication_hubImpl.hpp

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

modules/SerialCommHub/main/serial_communication_hubImpl.hpp#L40 

struct member 'Conf::retries' is never used.
};

class serial_communication_hubImpl : public serial_communication_hubImplBase {
Expand Down Expand Up @@ -80,10 +82,15 @@

// ev@3370e4dd-95f4-47a9-aaec-ea76f34a66c9:v1
// insert your private definitions here
static constexpr uint8_t num_resends_on_error{3};
types::serial_comm_hub_requests::Result
perform_modbus_request(uint8_t device_address, tiny_modbus::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>());

tiny_modbus::TinyModbusRTU modbus;

std::mutex serial_mutex;
bool system_error_logged{false};

Check notice on line 93 in modules/SerialCommHub/main/serial_communication_hubImpl.hpp

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

modules/SerialCommHub/main/serial_communication_hubImpl.hpp#L93 

class member 'serial_communication_hubImpl::system_error_logged' is never used.
// ev@3370e4dd-95f4-47a9-aaec-ea76f34a66c9:v1
};

Expand Down
6 changes: 6 additions & 0 deletions modules/SerialCommHub/manifest.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ provides:
description: Timeout in ms for subsequent packets.
type: integer
default: 100
retries:
description: Count of retries in case of error in Modbus query.
type: integer
minimum: 0
maximum: 10
default: 2
metadata:
license: https://opensource.org/licenses/Apache-2.0
authors:
Expand Down
Loading
Loading