PR-790 Address first set of code review feedback

This commit is contained in:
Garret Fick 2019-11-26 09:36:49 -05:00
parent 75d65b1605
commit 100302f8e1
3 changed files with 54 additions and 46 deletions

View File

@ -71,9 +71,9 @@ inline bool ini_matches(const char* section_expected,
/// This maximum is designed to prevent unintentionally allocating far more
/// space than would be expected under normal use.
/// @return 0 if there is a match, otherwise non-zero.
inline int strcmp_with_id(const char* name, const char* expected,
std::uint8_t max_index,
std::uint8_t* index) {
inline int strcmp_with_index(const char* name, const char* expected,
std::uint8_t max_index,
std::uint8_t* index) {
size_t expected_len = strlen(expected);
int ret = strncmp(name, expected, expected_len);
if (ret != 0) {

View File

@ -75,12 +75,12 @@ std::int16_t oplc::modbusm::common_cfg_handler(
{
const uint8_t max = MODBUS_MASTER_MAX - 1;
uint8_t index;
if (oplc::strcmp_with_id(name, "name", max, &index) == 0)
if (oplc::strcmp_with_index(name, "name", max, &index) == 0)
{
strncpy(masters[index].name, value, MASTER_ITEM_SIZE);
masters[index].name[MASTER_ITEM_SIZE - 1] = '\0';
}
else if (oplc::strcmp_with_id(name, "protocol", max, &index) == 0)
else if (oplc::strcmp_with_index(name, "protocol", max, &index) == 0)
{
if (strcmp(value, "tcp") == 0)
{
@ -95,7 +95,7 @@ std::int16_t oplc::modbusm::common_cfg_handler(
spdlog::warn("Unknown protocol configuration value {}", value);
}
}
else if (oplc::strcmp_with_id(name, "backoff_strategy", max, &index) == 0)
else if (oplc::strcmp_with_index(name, "backoff_strategy", max, &index) == 0)
{
if (strcmp(value, "linear_bounded") == 0)
{
@ -106,40 +106,40 @@ std::int16_t oplc::modbusm::common_cfg_handler(
spdlog::warn("Unknown backoff strategy configuration value {}", value);
}
}
else if (oplc::strcmp_with_id(name, "slave_id", max, &index) == 0)
else if (oplc::strcmp_with_index(name, "slave_id", max, &index) == 0)
{
masters[index].slave_id = atoi(value);
}
else if (oplc::strcmp_with_id(name, "response_timeout", max, &index) == 0)
else if (oplc::strcmp_with_index(name, "response_timeout", max, &index) == 0)
{
masters[index].response_timeout = chrono::microseconds(atoi(value));
}
else if (oplc::strcmp_with_id(name, "poll_interval", max, &index) == 0)
else if (oplc::strcmp_with_index(name, "poll_interval", max, &index) == 0)
{
masters[index].poll_interval = chrono::microseconds(atoi(value));
}
else if (oplc::strcmp_with_id(name, "ip_address", max, &index) == 0)
else if (oplc::strcmp_with_index(name, "ip_address", max, &index) == 0)
{
strncpy(masters[index].ip_address, value, MASTER_ITEM_SIZE);
masters[index].ip_address[MASTER_ITEM_SIZE - 1] = '\0';
}
else if (oplc::strcmp_with_id(name, "ip_port", max, &index) == 0)
else if (oplc::strcmp_with_index(name, "ip_port", max, &index) == 0)
{
masters[index].ip_port = atoi(value);
}
else if (oplc::strcmp_with_id(name, "rtu_baud_rate", max, &index) == 0)
else if (oplc::strcmp_with_index(name, "rtu_baud_rate", max, &index) == 0)
{
masters[index].rtu_baud_rate = atoi(value);
}
else if (oplc::strcmp_with_id(name, "rtu_parity", max, &index) == 0)
else if (oplc::strcmp_with_index(name, "rtu_parity", max, &index) == 0)
{
masters[index].rtu_parity = atoi(value);
}
else if (oplc::strcmp_with_id(name, "rtu_data_bit", max, &index) == 0)
else if (oplc::strcmp_with_index(name, "rtu_data_bit", max, &index) == 0)
{
masters[index].rtu_data_bit = atoi(value);
}
else if (oplc::strcmp_with_id(name, "rtu_stop_bit", max, &index) == 0)
else if (oplc::strcmp_with_index(name, "rtu_stop_bit", max, &index) == 0)
{
masters[index].rtu_stop_bit = atoi(value);
}

View File

@ -100,43 +100,43 @@ int cfg_handler(void* user_data, const char* section,
{
// It was already handled
}
else if (oplc::strcmp_with_id(name, "discrete_inputs_start", max, &index) == 0)
else if (oplc::strcmp_with_index(name, "discrete_inputs_start", max, &index) == 0)
{
mappings->at(index).discrete_inputs.start_address = atoi(value);
}
else if (oplc::strcmp_with_id(name, "discrete_inputs_size", max, &index) == 0)
else if (oplc::strcmp_with_index(name, "discrete_inputs_size", max, &index) == 0)
{
mappings->at(index).discrete_inputs.num_regs = atoi(value);
}
else if (oplc::strcmp_with_id(name, "coils_start", max, &index) == 0)
else if (oplc::strcmp_with_index(name, "coils_start", max, &index) == 0)
{
mappings->at(index).coils.start_address = atoi(value);
}
else if (oplc::strcmp_with_id(name, "coils_size", max, &index) == 0)
else if (oplc::strcmp_with_index(name, "coils_size", max, &index) == 0)
{
mappings->at(index).coils.num_regs = atoi(value);
}
else if (oplc::strcmp_with_id(name, "input_registers_start", max, &index) == 0)
else if (oplc::strcmp_with_index(name, "input_registers_start", max, &index) == 0)
{
mappings->at(index).input_registers.start_address = atoi(value);
}
else if (oplc::strcmp_with_id(name, "input_registers_size", max, &index) == 0)
else if (oplc::strcmp_with_index(name, "input_registers_size", max, &index) == 0)
{
mappings->at(index).input_registers.num_regs = atoi(value);
}
else if (oplc::strcmp_with_id(name, "holding_registers_read_start", max, &index) == 0)
else if (oplc::strcmp_with_index(name, "holding_registers_read_start", max, &index) == 0)
{
mappings->at(index).holding_read_registers.start_address = atoi(value);
}
else if (oplc::strcmp_with_id(name, "holding_registers_read_size", max, &index) == 0)
else if (oplc::strcmp_with_index(name, "holding_registers_read_size", max, &index) == 0)
{
mappings->at(index).holding_read_registers.num_regs = atoi(value);
}
else if (oplc::strcmp_with_id(name, "holding_registers_start", max, &index) == 0)
else if (oplc::strcmp_with_index(name, "holding_registers_start", max, &index) == 0)
{
mappings->at(index).holding_registers.start_address = atoi(value);
}
else if (oplc::strcmp_with_id(name, "holding_registers_size", max, &index) == 0)
else if (oplc::strcmp_with_index(name, "holding_registers_size", max, &index) == 0)
{
mappings->at(index).holding_registers.num_regs = atoi(value);
}
@ -334,27 +334,11 @@ void* oplc::modbusm::modbus_master_indexed_poll(void* args)
return 0;
}
/// Run the modbus master. This function does not return until
/// this service is terminated.
void modbus_master_run(oplc::config_stream& cfg_stream,
const char* cfg_overrides,
const GlueVariablesBinding& bindings,
volatile bool& run)
/// Assign the mappings from our bindings according to the strategy
/// that this master expect. In general, we map items starting from
/// 100 to the modbus master.
void assign_mappings(const GlueVariablesBinding& bindings)
{
// Read the configuration information for the masters
array<MasterConfig, MODBUS_MASTER_MAX> master_defs;
array<IndexedMapping, MODBUS_MASTER_MAX> mapping_defs;
array<pthread_t, MODBUS_MASTER_MAX> threads;
IndexedMasterConfig config
{
.masters = &master_defs,
.mappings = &mapping_defs,
};
ini_parse_stream(oplc::istream_fgets, cfg_stream.get(),
cfg_handler, &config);
cfg_stream.reset(nullptr);
// Initialize comm error counter. For historical reasons,
// this is defined to be at index 1026 in the IEC_LINT with
// the memory storage class.
@ -407,6 +391,29 @@ void modbus_master_run(oplc::config_stream& cfg_stream,
}
}
}
}
/// Run the modbus master. This function does not return until
/// this service is terminated.
void modbus_master_run(oplc::config_stream& cfg_stream,
const char* cfg_overrides,
const GlueVariablesBinding& bindings,
volatile bool& run)
{
// Read the configuration information for the masters
array<MasterConfig, MODBUS_MASTER_MAX> master_defs;
array<IndexedMapping, MODBUS_MASTER_MAX> mapping_defs;
array<pthread_t, MODBUS_MASTER_MAX> threads;
IndexedMasterConfig config
{
.masters = &master_defs,
.mappings = &mapping_defs,
};
ini_parse_stream(oplc::istream_fgets, cfg_stream.get(),
cfg_handler, &config);
cfg_stream.reset(nullptr);
assign_mappings(bindings);
// Create a thread for polling each master. We use multiple threads
// so that communication timeouts related to one master will not
@ -436,7 +443,8 @@ void modbus_master_run(oplc::config_stream& cfg_stream,
};
int ret = pthread_create(&threads[index], nullptr,
oplc::modbusm::modbus_master_indexed_poll, master_args);
oplc::modbusm::modbus_master_indexed_poll,
master_args);
if (ret == 0)
{
pthread_detach(threads[index]);