Commit a2e41db3862886d5787910f8c59765b5db1bbf34
1 parent
12753875
Renamed modbus_read_and_write_registers to modbus_write_and_read_registers
The function name was confusing because the write operation is performed before the read. Take care to swap the arguments in the migration process.
Showing
7 changed files
with
39 additions
and
34 deletions
NEWS
| @@ -6,6 +6,10 @@ libmodbus 2.9.5 (2011-06-XX) | @@ -6,6 +6,10 @@ libmodbus 2.9.5 (2011-06-XX) | ||
| 6 | by Tobias Doerffel, Florian octo Forster and Hannu Vuolasaho. | 6 | by Tobias Doerffel, Florian octo Forster and Hannu Vuolasaho. |
| 7 | - Enable RS485 support only when available | 7 | - Enable RS485 support only when available |
| 8 | - Export modbus_set/get_serial_mode functions on all platforms | 8 | - Export modbus_set/get_serial_mode functions on all platforms |
| 9 | +- API change for read/write multiple registers function: | ||
| 10 | + * modbus_read_and_write_registers -> modbus_write_and_read_registers | ||
| 11 | + The function name was confusing because the write operation is performed | ||
| 12 | + before the read. Take care to swap the arguments in the migration process. | ||
| 9 | 13 | ||
| 10 | libmodbus 2.9.4 (2011-06-05) | 14 | libmodbus 2.9.4 (2011-06-05) |
| 11 | ============================ | 15 | ============================ |
src/modbus-private.h
| @@ -61,7 +61,7 @@ MODBUS_BEGIN_DECLS | @@ -61,7 +61,7 @@ MODBUS_BEGIN_DECLS | ||
| 61 | #define _FC_WRITE_MULTIPLE_COILS 0x0F | 61 | #define _FC_WRITE_MULTIPLE_COILS 0x0F |
| 62 | #define _FC_WRITE_MULTIPLE_REGISTERS 0x10 | 62 | #define _FC_WRITE_MULTIPLE_REGISTERS 0x10 |
| 63 | #define _FC_REPORT_SLAVE_ID 0x11 | 63 | #define _FC_REPORT_SLAVE_ID 0x11 |
| 64 | -#define _FC_READ_AND_WRITE_REGISTERS 0x17 | 64 | +#define _FC_WRITE_AND_READ_REGISTERS 0x17 |
| 65 | 65 | ||
| 66 | typedef enum { | 66 | typedef enum { |
| 67 | _MODBUS_BACKEND_TYPE_RTU=0, | 67 | _MODBUS_BACKEND_TYPE_RTU=0, |
src/modbus.c
| @@ -138,7 +138,7 @@ static unsigned int compute_response_length_from_request(modbus_t *ctx, uint8_t | @@ -138,7 +138,7 @@ static unsigned int compute_response_length_from_request(modbus_t *ctx, uint8_t | ||
| 138 | length = 2 + (nb / 8) + ((nb % 8) ? 1 : 0); | 138 | length = 2 + (nb / 8) + ((nb % 8) ? 1 : 0); |
| 139 | } | 139 | } |
| 140 | break; | 140 | break; |
| 141 | - case _FC_READ_AND_WRITE_REGISTERS: | 141 | + case _FC_WRITE_AND_READ_REGISTERS: |
| 142 | case _FC_READ_HOLDING_REGISTERS: | 142 | case _FC_READ_HOLDING_REGISTERS: |
| 143 | case _FC_READ_INPUT_REGISTERS: | 143 | case _FC_READ_INPUT_REGISTERS: |
| 144 | /* Header + 2 * nb values */ | 144 | /* Header + 2 * nb values */ |
| @@ -254,7 +254,7 @@ static uint8_t compute_meta_length_after_function(int function, | @@ -254,7 +254,7 @@ static uint8_t compute_meta_length_after_function(int function, | ||
| 254 | } else if (function == _FC_WRITE_MULTIPLE_COILS || | 254 | } else if (function == _FC_WRITE_MULTIPLE_COILS || |
| 255 | function == _FC_WRITE_MULTIPLE_REGISTERS) { | 255 | function == _FC_WRITE_MULTIPLE_REGISTERS) { |
| 256 | length = 5; | 256 | length = 5; |
| 257 | - } else if (function == _FC_READ_AND_WRITE_REGISTERS) { | 257 | + } else if (function == _FC_WRITE_AND_READ_REGISTERS) { |
| 258 | length = 9; | 258 | length = 9; |
| 259 | } else { | 259 | } else { |
| 260 | /* _FC_READ_EXCEPTION_STATUS, _FC_REPORT_SLAVE_ID */ | 260 | /* _FC_READ_EXCEPTION_STATUS, _FC_REPORT_SLAVE_ID */ |
| @@ -290,7 +290,7 @@ static int compute_data_length_after_meta(modbus_t *ctx, uint8_t *msg, | @@ -290,7 +290,7 @@ static int compute_data_length_after_meta(modbus_t *ctx, uint8_t *msg, | ||
| 290 | case _FC_WRITE_MULTIPLE_REGISTERS: | 290 | case _FC_WRITE_MULTIPLE_REGISTERS: |
| 291 | length = msg[ctx->backend->header_length + 5]; | 291 | length = msg[ctx->backend->header_length + 5]; |
| 292 | break; | 292 | break; |
| 293 | - case _FC_READ_AND_WRITE_REGISTERS: | 293 | + case _FC_WRITE_AND_READ_REGISTERS: |
| 294 | length = msg[ctx->backend->header_length + 9]; | 294 | length = msg[ctx->backend->header_length + 9]; |
| 295 | break; | 295 | break; |
| 296 | default: | 296 | default: |
| @@ -300,7 +300,7 @@ static int compute_data_length_after_meta(modbus_t *ctx, uint8_t *msg, | @@ -300,7 +300,7 @@ static int compute_data_length_after_meta(modbus_t *ctx, uint8_t *msg, | ||
| 300 | /* MSG_CONFIRMATION */ | 300 | /* MSG_CONFIRMATION */ |
| 301 | if (function <= _FC_READ_INPUT_REGISTERS || | 301 | if (function <= _FC_READ_INPUT_REGISTERS || |
| 302 | function == _FC_REPORT_SLAVE_ID || | 302 | function == _FC_REPORT_SLAVE_ID || |
| 303 | - function == _FC_READ_AND_WRITE_REGISTERS) { | 303 | + function == _FC_WRITE_AND_READ_REGISTERS) { |
| 304 | length = msg[ctx->backend->header_length + 1]; | 304 | length = msg[ctx->backend->header_length + 1]; |
| 305 | } else { | 305 | } else { |
| 306 | length = 0; | 306 | length = 0; |
| @@ -526,7 +526,7 @@ static int check_confirmation(modbus_t *ctx, uint8_t *req, | @@ -526,7 +526,7 @@ static int check_confirmation(modbus_t *ctx, uint8_t *req, | ||
| 526 | req_nb_value = (req_nb_value / 8) + ((req_nb_value % 8) ? 1 : 0); | 526 | req_nb_value = (req_nb_value / 8) + ((req_nb_value % 8) ? 1 : 0); |
| 527 | rsp_nb_value = rsp[offset + 1]; | 527 | rsp_nb_value = rsp[offset + 1]; |
| 528 | break; | 528 | break; |
| 529 | - case _FC_READ_AND_WRITE_REGISTERS: | 529 | + case _FC_WRITE_AND_READ_REGISTERS: |
| 530 | case _FC_READ_HOLDING_REGISTERS: | 530 | case _FC_READ_HOLDING_REGISTERS: |
| 531 | case _FC_READ_INPUT_REGISTERS: | 531 | case _FC_READ_INPUT_REGISTERS: |
| 532 | /* Read functions 1 value = 2 bytes */ | 532 | /* Read functions 1 value = 2 bytes */ |
| @@ -858,7 +858,7 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, | @@ -858,7 +858,7 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, | ||
| 858 | return -1; | 858 | return -1; |
| 859 | break; | 859 | break; |
| 860 | 860 | ||
| 861 | - case _FC_READ_AND_WRITE_REGISTERS: { | 861 | + case _FC_WRITE_AND_READ_REGISTERS: { |
| 862 | int nb = (req[offset + 3] << 8) + req[offset + 4]; | 862 | int nb = (req[offset + 3] << 8) + req[offset + 4]; |
| 863 | uint16_t address_write = (req[offset + 5] << 8) + req[offset + 6]; | 863 | uint16_t address_write = (req[offset + 5] << 8) + req[offset + 6]; |
| 864 | int nb_write = (req[offset + 7] << 8) + req[offset + 8]; | 864 | int nb_write = (req[offset + 7] << 8) + req[offset + 8]; |
| @@ -867,7 +867,7 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, | @@ -867,7 +867,7 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, | ||
| 867 | (address_write + nb_write) > mb_mapping->nb_registers) { | 867 | (address_write + nb_write) > mb_mapping->nb_registers) { |
| 868 | if (ctx->debug) { | 868 | if (ctx->debug) { |
| 869 | fprintf(stderr, | 869 | fprintf(stderr, |
| 870 | - "Illegal data read address %0X or write address %0X in read_and_write_registers\n", | 870 | + "Illegal data read address %0X or write address %0X write_and_read_registers\n", |
| 871 | address + nb, address_write + nb_write); | 871 | address + nb, address_write + nb_write); |
| 872 | } | 872 | } |
| 873 | rsp_length = response_exception(ctx, &sft, | 873 | rsp_length = response_exception(ctx, &sft, |
| @@ -1255,11 +1255,12 @@ int modbus_write_registers(modbus_t *ctx, int addr, int nb, const uint16_t *src) | @@ -1255,11 +1255,12 @@ int modbus_write_registers(modbus_t *ctx, int addr, int nb, const uint16_t *src) | ||
| 1255 | return rc; | 1255 | return rc; |
| 1256 | } | 1256 | } |
| 1257 | 1257 | ||
| 1258 | -/* Read multiple registers from remote device to dest array and write multiple | ||
| 1259 | - registers to remote device from data array. */ | ||
| 1260 | -int modbus_read_and_write_registers(modbus_t *ctx, | ||
| 1261 | - int read_addr, int read_nb, uint16_t *dest, | ||
| 1262 | - int write_addr, int write_nb, const uint16_t *data) | 1258 | +/* Write multiple registers from src array to remote device and read multiple |
| 1259 | + registers from remote device to dest array. */ | ||
| 1260 | +int modbus_write_and_read_registers(modbus_t *ctx, | ||
| 1261 | + int write_addr, int write_nb, const uint16_t *src, | ||
| 1262 | + int read_addr, int read_nb, uint16_t *dest) | ||
| 1263 | + | ||
| 1263 | { | 1264 | { |
| 1264 | int rc; | 1265 | int rc; |
| 1265 | int req_length; | 1266 | int req_length; |
| @@ -1268,27 +1269,27 @@ int modbus_read_and_write_registers(modbus_t *ctx, | @@ -1268,27 +1269,27 @@ int modbus_read_and_write_registers(modbus_t *ctx, | ||
| 1268 | uint8_t req[MAX_MESSAGE_LENGTH]; | 1269 | uint8_t req[MAX_MESSAGE_LENGTH]; |
| 1269 | uint8_t rsp[MAX_MESSAGE_LENGTH]; | 1270 | uint8_t rsp[MAX_MESSAGE_LENGTH]; |
| 1270 | 1271 | ||
| 1271 | - if (read_nb > MODBUS_MAX_READ_REGISTERS) { | 1272 | + if (write_nb > MODBUS_MAX_RW_WRITE_REGISTERS) { |
| 1272 | if (ctx->debug) { | 1273 | if (ctx->debug) { |
| 1273 | fprintf(stderr, | 1274 | fprintf(stderr, |
| 1274 | - "ERROR Too many registers requested (%d > %d)\n", | ||
| 1275 | - read_nb, MODBUS_MAX_READ_REGISTERS); | 1275 | + "ERROR Too many registers to write (%d > %d)\n", |
| 1276 | + write_nb, MODBUS_MAX_RW_WRITE_REGISTERS); | ||
| 1276 | } | 1277 | } |
| 1277 | errno = EMBMDATA; | 1278 | errno = EMBMDATA; |
| 1278 | return -1; | 1279 | return -1; |
| 1279 | } | 1280 | } |
| 1280 | 1281 | ||
| 1281 | - if (write_nb > MODBUS_MAX_RW_WRITE_REGISTERS) { | 1282 | + if (read_nb > MODBUS_MAX_READ_REGISTERS) { |
| 1282 | if (ctx->debug) { | 1283 | if (ctx->debug) { |
| 1283 | fprintf(stderr, | 1284 | fprintf(stderr, |
| 1284 | - "ERROR Too many registers to write (%d > %d)\n", | ||
| 1285 | - write_nb, MODBUS_MAX_RW_WRITE_REGISTERS); | 1285 | + "ERROR Too many registers requested (%d > %d)\n", |
| 1286 | + read_nb, MODBUS_MAX_READ_REGISTERS); | ||
| 1286 | } | 1287 | } |
| 1287 | errno = EMBMDATA; | 1288 | errno = EMBMDATA; |
| 1288 | return -1; | 1289 | return -1; |
| 1289 | } | 1290 | } |
| 1290 | req_length = ctx->backend->build_request_basis(ctx, | 1291 | req_length = ctx->backend->build_request_basis(ctx, |
| 1291 | - _FC_READ_AND_WRITE_REGISTERS, | 1292 | + _FC_WRITE_AND_READ_REGISTERS, |
| 1292 | read_addr, read_nb, req); | 1293 | read_addr, read_nb, req); |
| 1293 | 1294 | ||
| 1294 | req[req_length++] = write_addr >> 8; | 1295 | req[req_length++] = write_addr >> 8; |
| @@ -1299,8 +1300,8 @@ int modbus_read_and_write_registers(modbus_t *ctx, | @@ -1299,8 +1300,8 @@ int modbus_read_and_write_registers(modbus_t *ctx, | ||
| 1299 | req[req_length++] = byte_count; | 1300 | req[req_length++] = byte_count; |
| 1300 | 1301 | ||
| 1301 | for (i = 0; i < write_nb; i++) { | 1302 | for (i = 0; i < write_nb; i++) { |
| 1302 | - req[req_length++] = data[i] >> 8; | ||
| 1303 | - req[req_length++] = data[i] & 0x00FF; | 1303 | + req[req_length++] = src[i] >> 8; |
| 1304 | + req[req_length++] = src[i] & 0x00FF; | ||
| 1304 | } | 1305 | } |
| 1305 | 1306 | ||
| 1306 | rc = send_msg(ctx, req, req_length); | 1307 | rc = send_msg(ctx, req, req_length); |
src/modbus.h
| @@ -173,9 +173,9 @@ int modbus_write_bit(modbus_t *ctx, int coil_addr, int status); | @@ -173,9 +173,9 @@ int modbus_write_bit(modbus_t *ctx, int coil_addr, int status); | ||
| 173 | int modbus_write_register(modbus_t *ctx, int reg_addr, int value); | 173 | int modbus_write_register(modbus_t *ctx, int reg_addr, int value); |
| 174 | int modbus_write_bits(modbus_t *ctx, int addr, int nb, const uint8_t *data); | 174 | int modbus_write_bits(modbus_t *ctx, int addr, int nb, const uint8_t *data); |
| 175 | int modbus_write_registers(modbus_t *ctx, int addr, int nb, const uint16_t *data); | 175 | int modbus_write_registers(modbus_t *ctx, int addr, int nb, const uint16_t *data); |
| 176 | -int modbus_read_and_write_registers(modbus_t *ctx, int read_addr, | ||
| 177 | - int read_nb, uint16_t *dest, int write_addr, | ||
| 178 | - int write_nb, const uint16_t *data); | 176 | +int modbus_write_and_read_registers(modbus_t *ctx, int write_addr, int write_nb, |
| 177 | + const uint16_t *src, int read_addr, int read_nb, | ||
| 178 | + uint16_t *dest); | ||
| 179 | int modbus_report_slave_id(modbus_t *ctx, uint8_t *dest); | 179 | int modbus_report_slave_id(modbus_t *ctx, uint8_t *dest); |
| 180 | 180 | ||
| 181 | modbus_mapping_t* modbus_mapping_new(int nb_coil_status, int nb_input_status, | 181 | modbus_mapping_t* modbus_mapping_new(int nb_coil_status, int nb_input_status, |
tests/bandwidth-client.c
| @@ -174,7 +174,7 @@ int main(int argc, char *argv[]) | @@ -174,7 +174,7 @@ int main(int argc, char *argv[]) | ||
| 174 | nb_points = MODBUS_MAX_RW_WRITE_REGISTERS; | 174 | nb_points = MODBUS_MAX_RW_WRITE_REGISTERS; |
| 175 | start = gettime_ms(); | 175 | start = gettime_ms(); |
| 176 | for (i=0; i<n_loop; i++) { | 176 | for (i=0; i<n_loop; i++) { |
| 177 | - rc = modbus_read_and_write_registers(ctx, | 177 | + rc = modbus_write_and_read_registers(ctx, |
| 178 | 0, nb_points, tab_reg, | 178 | 0, nb_points, tab_reg, |
| 179 | 0, nb_points, tab_reg); | 179 | 0, nb_points, tab_reg); |
| 180 | if (rc == -1) { | 180 | if (rc == -1) { |
tests/random-test-client.c
| @@ -195,9 +195,9 @@ int main(void) | @@ -195,9 +195,9 @@ int main(void) | ||
| 195 | } | 195 | } |
| 196 | } | 196 | } |
| 197 | /* R/W MULTIPLE REGISTERS */ | 197 | /* R/W MULTIPLE REGISTERS */ |
| 198 | - rc = modbus_read_and_write_registers(ctx, | ||
| 199 | - addr, nb, tab_rp_registers, | ||
| 200 | - addr, nb, tab_rw_rq_registers); | 198 | + rc = modbus_write_and_read_registers(ctx, |
| 199 | + addr, nb, tab_rw_rq_registers, | ||
| 200 | + addr, nb, tab_rp_registers); | ||
| 201 | if (rc != nb) { | 201 | if (rc != nb) { |
| 202 | printf("ERROR modbus_read_and_write_registers (%d)\n", rc); | 202 | printf("ERROR modbus_read_and_write_registers (%d)\n", rc); |
| 203 | printf("Address = %d, nb = %d\n", addr, nb); | 203 | printf("Address = %d, nb = %d\n", addr, nb); |
tests/unit-test-client.c
| @@ -269,13 +269,13 @@ int main(int argc, char *argv[]) | @@ -269,13 +269,13 @@ int main(int argc, char *argv[]) | ||
| 269 | /* Write registers to zero from tab_rp_registers and store read registers | 269 | /* Write registers to zero from tab_rp_registers and store read registers |
| 270 | into tab_rp_registers. So the read registers must set to 0, except the | 270 | into tab_rp_registers. So the read registers must set to 0, except the |
| 271 | first one because there is an offset of 1 register on write. */ | 271 | first one because there is an offset of 1 register on write. */ |
| 272 | - rc = modbus_read_and_write_registers(ctx, | ||
| 273 | - UT_REGISTERS_ADDRESS, UT_REGISTERS_NB, | 272 | + rc = modbus_write_and_read_registers(ctx, |
| 273 | + UT_REGISTERS_ADDRESS + 1, UT_REGISTERS_NB - 1, | ||
| 274 | tab_rp_registers, | 274 | tab_rp_registers, |
| 275 | - UT_REGISTERS_ADDRESS + 1, | ||
| 276 | - UT_REGISTERS_NB - 1, | 275 | + UT_REGISTERS_ADDRESS, |
| 276 | + UT_REGISTERS_NB, | ||
| 277 | tab_rp_registers); | 277 | tab_rp_registers); |
| 278 | - printf("4/5 modbus_read_and_write_registers: "); | 278 | + printf("4/5 modbus_write_and_read_registers: "); |
| 279 | if (rc != UT_REGISTERS_NB) { | 279 | if (rc != UT_REGISTERS_NB) { |
| 280 | printf("FAILED (nb points %d != %d)\n", rc, UT_REGISTERS_NB); | 280 | printf("FAILED (nb points %d != %d)\n", rc, UT_REGISTERS_NB); |
| 281 | goto close; | 281 | goto close; |