Skip to content

Commit

Permalink
Merge branch 'bugfix/fix_coil_start_bit_readout' into 'master'
Browse files Browse the repository at this point in the history 
Modbus coils do not shift the readout by n%8  (merge PR espressif#35)

Closes IDFGH-10998

See merge request idf/esp-modbus!50
  • Loading branch information
@alisitsyn
alisitsyn committed Sep 29, 2023
2 parents aa93eec + 8950d1c commit e24ea4e
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 58 deletions.
8 changes: 4 additions & 4 deletions freemodbus/serial_master/modbus_controller/mbc_serial_master.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,7 @@ eMBErrorCode eMBRegCoilsCBSerialMaster(UCHAR* pucRegBuffer, USHORT usAddress,
switch (eMode) {
case MB_REG_WRITE:
while (usCoils > 0) {
UCHAR ucResult = xMBUtilGetBits((UCHAR*)pucRegCoilsBuf, iRegIndex, 1);
UCHAR ucResult = xMBUtilGetBits(pucRegCoilsBuf, iRegIndex - (usAddress % 8), 1);
xMBUtilSetBits(pucRegBuffer, iRegIndex - (usAddress % 8) , 1, ucResult);
iRegIndex++;
usCoils--;
Expand All @@ -575,7 +575,7 @@ eMBErrorCode eMBRegCoilsCBSerialMaster(UCHAR* pucRegBuffer, USHORT usAddress,
case MB_REG_READ:
while (usCoils > 0) {
UCHAR ucResult = xMBUtilGetBits(pucRegBuffer, iRegIndex - (usAddress % 8), 1);
xMBUtilSetBits((uint8_t*)pucRegCoilsBuf, iRegIndex, 1, ucResult);
xMBUtilSetBits(pucRegCoilsBuf, iRegIndex - (usAddress % 8), 1, ucResult);
iRegIndex++;
usCoils--;
}
Expand Down Expand Up @@ -621,15 +621,15 @@ eMBErrorCode eMBRegDiscreteCBSerialMaster(UCHAR * pucRegBuffer, USHORT usAddress
iRegBitIndex = (USHORT)(usAddress) % 8; // Get bit index
while (iNReg > 1)
{
xMBUtilSetBits(pucDiscreteInputBuf++, iRegBitIndex, 8, *pucRegBuffer++);
xMBUtilSetBits(pucDiscreteInputBuf++, iRegBitIndex - (usAddress % 8), 8, *pucRegBuffer++);
iNReg--;
}
// last discrete
usNDiscrete = usNDiscrete % 8;
// xMBUtilSetBits has bug when ucNBits is zero
if (usNDiscrete != 0)
{
xMBUtilSetBits(pucDiscreteInputBuf, iRegBitIndex, usNDiscrete, *pucRegBuffer++);
xMBUtilSetBits(pucDiscreteInputBuf, iRegBitIndex - (usAddress % 8), usNDiscrete, *pucRegBuffer++);
}
} else {
eStatus = MB_ENOREG;
Expand Down
8 changes: 4 additions & 4 deletions freemodbus/tcp_master/modbus_controller/mbc_tcp_master.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -685,7 +685,7 @@ eMBErrorCode eMBRegCoilsCBTcpMaster(UCHAR *pucRegBuffer, USHORT usAddress,
switch (eMode) {
case MB_REG_WRITE:
while (usCoils > 0) {
UCHAR ucResult = xMBUtilGetBits((UCHAR*)pucRegCoilsBuf, iRegIndex, 1);
UCHAR ucResult = xMBUtilGetBits(pucRegCoilsBuf, iRegIndex - (usAddress % 8), 1);
xMBUtilSetBits(pucRegBuffer, iRegIndex - (usAddress % 8) , 1, ucResult);
iRegIndex++;
usCoils--;
Expand All @@ -694,7 +694,7 @@ eMBErrorCode eMBRegCoilsCBTcpMaster(UCHAR *pucRegBuffer, USHORT usAddress,
case MB_REG_READ:
while (usCoils > 0) {
UCHAR ucResult = xMBUtilGetBits(pucRegBuffer, iRegIndex - (usAddress % 8), 1);
xMBUtilSetBits((uint8_t*)pucRegCoilsBuf, iRegIndex, 1, ucResult);
xMBUtilSetBits(pucRegCoilsBuf, iRegIndex - (usAddress % 8), 1, ucResult);
iRegIndex++;
usCoils--;
}
Expand Down Expand Up @@ -738,15 +738,15 @@ eMBErrorCode eMBRegDiscreteCBTcpMaster(UCHAR * pucRegBuffer, USHORT usAddress,
iRegBitIndex = (USHORT)(usAddress) % 8; // Get bit index
while (iNReg > 1)
{
xMBUtilSetBits(pucDiscreteInputBuf++, iRegBitIndex, 8, *pucRegBuffer++);
xMBUtilSetBits(pucDiscreteInputBuf++, iRegBitIndex - (usAddress % 8), 8, *pucRegBuffer++);
iNReg--;
}
// last discrete
usNDiscrete = usNDiscrete % 8;
// xMBUtilSetBits has bug when ucNBits is zero
if (usNDiscrete != 0)
{
xMBUtilSetBits(pucDiscreteInputBuf, iRegBitIndex, usNDiscrete, *pucRegBuffer++);
xMBUtilSetBits(pucDiscreteInputBuf, iRegBitIndex - (usAddress % 8), usNDiscrete, *pucRegBuffer++);
}
} else {
eStatus = MB_ENOREG;
Expand Down
4 changes: 3 additions & 1 deletion test/mb_example_common/include/modbus_params.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ typedef struct
uint8_t discrete_input5:1;
uint8_t discrete_input6:1;
uint8_t discrete_input7:1;
uint8_t discrete_input_port1:8;
uint8_t discrete_input_port1;
uint8_t discrete_input_port2;
} discrete_reg_params_t;
#pragma pack(pop)

Expand All @@ -35,6 +36,7 @@ typedef struct
{
uint8_t coils_port0;
uint8_t coils_port1;
uint8_t coils_port2;
} coil_reg_params_t;
#pragma pack(pop)

Expand Down
70 changes: 42 additions & 28 deletions test/serial/mb_serial_master/main/master.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ enum {
CID_HOLD_TEST_REG,
CID_RELAY_P1,
CID_RELAY_P2,
CID_DISCR_P1,
CID_COUNT
};

Expand All @@ -74,7 +75,7 @@ enum {
const mb_parameter_descriptor_t device_parameters[] = {
// { CID, Param Name, Units, Modbus Slave Addr, Modbus Reg Type, Reg Start, Reg Size, Instance Offset, Data Type, Data Size, Parameter Options, Access Mode}
{ CID_INP_DATA_0, STR("Data_channel_0"), STR("Volts"), MB_DEVICE_ADDR1, MB_PARAM_INPUT, 0, 2,
INPUT_OFFSET(input_data0), PARAM_TYPE_FLOAT, 4, OPTS( -10, 10, 1 ), PAR_PERMS_READ_WRITE_TRIGGER },
INPUT_OFFSET(input_data0), PARAM_TYPE_FLOAT, 4, OPTS( -10, 10, 1 ), PAR_PERMS_READ_WRITE_TRIGGER },
{ CID_HOLD_DATA_0, STR("Humidity_1"), STR("%rH"), MB_DEVICE_ADDR1, MB_PARAM_HOLDING, 0, 2,
HOLD_OFFSET(holding_data0), PARAM_TYPE_FLOAT, 4, OPTS( 0, 100, 1 ), PAR_PERMS_READ_WRITE_TRIGGER },
{ CID_INP_DATA_1, STR("Temperature_1"), STR("C"), MB_DEVICE_ADDR1, MB_PARAM_INPUT, 2, 2,
Expand All @@ -87,10 +88,12 @@ const mb_parameter_descriptor_t device_parameters[] = {
HOLD_OFFSET(holding_data2), PARAM_TYPE_FLOAT, 4, OPTS( 0, 100, 1 ), PAR_PERMS_READ_WRITE_TRIGGER },
{ CID_HOLD_TEST_REG, STR("Test_regs"), STR("__"), MB_DEVICE_ADDR1, MB_PARAM_HOLDING, 10, 58,
HOLD_OFFSET(test_regs), PARAM_TYPE_ASCII, 116, OPTS( 0, 100, 1 ), PAR_PERMS_READ_WRITE_TRIGGER },
{ CID_RELAY_P1, STR("RelayP1"), STR("on/off"), MB_DEVICE_ADDR1, MB_PARAM_COIL, 0, 8,
COIL_OFFSET(coils_port0), PARAM_TYPE_U16, 2, OPTS( BIT1, 0, 0 ), PAR_PERMS_READ_WRITE_TRIGGER },
{ CID_RELAY_P2, STR("RelayP2"), STR("on/off"), MB_DEVICE_ADDR1, MB_PARAM_COIL, 8, 8,
COIL_OFFSET(coils_port1), PARAM_TYPE_U16, 2, OPTS( BIT0, 0, 0 ), PAR_PERMS_READ_WRITE_TRIGGER }
{ CID_RELAY_P1, STR("RelayP1"), STR("on/off"), MB_DEVICE_ADDR1, MB_PARAM_COIL, 2, 6,
COIL_OFFSET(coils_port0), PARAM_TYPE_U8, 1, OPTS( 0xAA, 0x15, 0 ), PAR_PERMS_READ_WRITE_TRIGGER },
{ CID_RELAY_P2, STR("RelayP2"), STR("on/off"), MB_DEVICE_ADDR1, MB_PARAM_COIL, 10, 6,
COIL_OFFSET(coils_port1), PARAM_TYPE_U8, 1, OPTS( 0x55, 0x2A, 0 ), PAR_PERMS_READ_WRITE_TRIGGER },
{ CID_DISCR_P1, STR("DiscreteInpP1"), STR("on/off"), MB_DEVICE_ADDR1, MB_PARAM_DISCRETE, 2, 7,
DISCR_OFFSET(discrete_input_port1), PARAM_TYPE_U8, 1, OPTS( 0xAA, 0x15, 0 ), PAR_PERMS_READ_WRITE_TRIGGER }
};

// Calculate number of parameters in the table
Expand Down Expand Up @@ -153,13 +156,13 @@ static void master_operation_func(void *arg)
(param_descriptor->cid == CID_HOLD_TEST_REG)) {
// Check for long array of registers of type PARAM_TYPE_ASCII
err = mbc_master_get_parameter(cid, (char*)param_descriptor->param_key,
(uint8_t*)temp_data_ptr, &type);
(uint8_t*)temp_data_ptr, &type);
if (err == ESP_OK) {
ESP_LOGI(TAG, "Characteristic #%d %s (%s) value = (0x%" PRIx32 ") read successful.",
(int)param_descriptor->cid,
(char*)param_descriptor->param_key,
(char*)param_descriptor->param_units,
*(uint32_t*)temp_data_ptr);
(int)param_descriptor->cid,
(char*)param_descriptor->param_key,
(char*)param_descriptor->param_units,
*(uint32_t*)temp_data_ptr);
// Initialize data of test array and write to slave
if (*(uint32_t*)temp_data_ptr != 0xAAAAAAAA) {
memset((void*)temp_data_ptr, 0xAA, param_descriptor->param_size);
Expand All @@ -168,32 +171,32 @@ static void master_operation_func(void *arg)
(uint8_t*)temp_data_ptr, &type);
if (err == ESP_OK) {
ESP_LOGI(TAG, "Characteristic #%d %s (%s) value = (0x%" PRIx32 "), write successful.",
(int)param_descriptor->cid,
(char*)param_descriptor->param_key,
(char*)param_descriptor->param_units,
*(uint32_t*)temp_data_ptr);
(int)param_descriptor->cid,
(char*)param_descriptor->param_key,
(char*)param_descriptor->param_units,
*(uint32_t*)temp_data_ptr);
} else {
ESP_LOGE(TAG, "Characteristic #%d (%s) write fail, err = 0x%x (%s).",
(int)param_descriptor->cid,
(char*)param_descriptor->param_key,
(int)err,
(char*)esp_err_to_name(err));
}
}
} else {
ESP_LOGE(TAG, "Characteristic #%d (%s) read fail, err = 0x%x (%s).",
(int)param_descriptor->cid,
(char*)param_descriptor->param_key,
(int)err,
(char*)esp_err_to_name(err));
}
}
} else {
ESP_LOGE(TAG, "Characteristic #%d (%s) read fail, err = 0x%x (%s).",
(int)param_descriptor->cid,
(char*)param_descriptor->param_key,
(int)err,
(char*)esp_err_to_name(err));
}
} else {
err = mbc_master_get_parameter(cid, (char*)param_descriptor->param_key,
(uint8_t*)&value, &type);
(uint8_t*)temp_data_ptr, &type);
if (err == ESP_OK) {
*(float*)temp_data_ptr = value;
if ((param_descriptor->mb_param_type == MB_PARAM_HOLDING) ||
(param_descriptor->mb_param_type == MB_PARAM_INPUT)) {
value = *(float*)temp_data_ptr;
ESP_LOGI(TAG, "Characteristic #%d %s (%s) value = %f (0x%" PRIx32 ") read successful.",
(int)param_descriptor->cid,
(char*)param_descriptor->param_key,
Expand All @@ -206,14 +209,25 @@ static void master_operation_func(void *arg)
break;
}
} else {
uint16_t state = *(uint16_t*)temp_data_ptr;
uint8_t state = *(uint8_t*)temp_data_ptr;
const char* rw_str = (state & param_descriptor->param_opts.opt1) ? "ON" : "OFF";
ESP_LOGI(TAG, "Characteristic #%d %s (%s) value = %s (0x%x) read successful.",
if ((state & param_descriptor->param_opts.opt2) == param_descriptor->param_opts.opt2) {
ESP_LOGI(TAG, "Characteristic #%d %s (%s) value = %s (0x%" PRIx8 ") read successful.",
(int)param_descriptor->cid,
(char*)param_descriptor->param_key,
(char*)param_descriptor->param_units,
(const char*)rw_str,
*(uint8_t*)temp_data_ptr);
} else {
ESP_LOGE(TAG, "Characteristic #%d %s (%s) value = %s (0x%" PRIx8 "), unexpected value.",
(int)param_descriptor->cid,
(char*)param_descriptor->param_key,
(char*)param_descriptor->param_units,
(const char*)rw_str,
*(uint16_t*)temp_data_ptr);
*(uint8_t*)temp_data_ptr);
alarm_state = true;
break;
}
if (state & param_descriptor->param_opts.opt1) {
alarm_state = true;
break;
Expand All @@ -230,7 +244,7 @@ static void master_operation_func(void *arg)
vTaskDelay(POLL_TIMEOUT_TICS); // timeout between polls
}
}
vTaskDelay(UPDATE_CIDS_TIMEOUT_TICS); //
vTaskDelay(UPDATE_CIDS_TIMEOUT_TICS);
}

if (alarm_state) {
Expand Down
Loading

0 comments on commit e24ea4e

Please sign in to comment.