From d022ff46c104a5f03779a431c546b3af0a1d74be Mon Sep 17 00:00:00 2001 From: Kamil Wcislo Date: Thu, 27 Apr 2017 16:19:34 +0200 Subject: [PATCH 01/21] Add write_file_record and read_file_record functions --- src/modbus.c | 125 +++++++++++++++++++++++++++++++++++++++++++++++++++ src/modbus.h | 9 ++++ 2 files changed, 134 insertions(+) diff --git a/src/modbus.c b/src/modbus.c index 41a421359..e8b87ce06 100644 --- a/src/modbus.c +++ b/src/modbus.c @@ -600,6 +600,12 @@ static int check_confirmation(modbus_t *ctx, uint8_t *req, /* Report slave ID (bytes received) */ req_nb_value = rsp_nb_value = rsp[offset + 1]; break; + case MODBUS_FC_READ_FILE_RECORD: + req_nb_value = (req[offset + 7] << 8) + req[offset + 8]; + rsp_nb_value = (rsp[offset + 2] - 1) / 2; + case MODBUS_FC_WRITE_FILE_RECORD: + req_nb_value = (req[offset + 7] << 8) + req[offset + 8]; + rsp_nb_value = (rsp[offset + 7] << 8) + rsp[offset + 8]; default: /* 1 Write functions & others */ req_nb_value = rsp_nb_value = 1; @@ -1236,6 +1242,67 @@ int modbus_read_input_registers(modbus_t *ctx, int addr, int nb, return status; } +int modbus_read_file_record(modbus_t *ctx, int addr, int sub_addr, int nb, uint16_t *dest) +{ + int rc; + int req_length; + uint8_t req[_MIN_REQ_LENGTH]; + uint8_t rsp[MAX_MESSAGE_LENGTH]; + + if (ctx == NULL) { + errno = EINVAL; + return -1; + } + + if (sub_addr > MODBUS_MAX_FILE_RECORD_NUMBER) { + if (ctx->debug) { + fprintf(stderr, + "ERROR Too big file record number (%d > %d)\n", + sub_addr, MODBUS_MAX_FILE_RECORD_NUMBER); + } + errno = EMBMDATA; + return -1; + } + + req_length = ctx->backend->build_request_basis(ctx, + MODBUS_FC_READ_FILE_RECORD, + 0, 0, req); + + req_length -= 4; + req[req_length++] = 7; // one request, so 7 bytes + req[req_length++] = 6; + req[req_length++] = addr >> 8; + req[req_length++] = addr & 0x00ff; + req[req_length++] = sub_addr >> 8; + req[req_length++] = sub_addr & 0x00ff; + req[req_length++] = nb >> 8; + req[req_length++] = nb * 0x00ff; + + rc = send_msg(ctx, req, req_length); + if (rc > 0) { + int offset; + int i; + + rc = _modbus_receive_msg(ctx, rsp, MSG_CONFIRMATION); + if (rc == -1) + return -1; + + rc = check_confirmation(ctx, req, rsp, rc); + if (rc == -1) + return -1; + + offset = ctx->backend->header_length; + + for (i = 0; i < rc; i++) { + + dest[i] = (rsp[offset + 4 + (i << 1)] << 8) | + rsp[offset + 5 + (i << 1)]; + } + } + + return rc; +} + /* Write a value to the specified register of the remote device. Used by write_bit and write_register */ static int write_single(modbus_t *ctx, int function, int addr, const uint16_t value) @@ -1401,6 +1468,64 @@ int modbus_write_registers(modbus_t *ctx, int addr, int nb, const uint16_t *src) return rc; } +int modbus_write_file_record(modbus_t *ctx, int addr, int sub_addr, int nb, const uint16_t *src) +{ + int rc; + int i; + int req_length; + int byte_count; + uint8_t req[MAX_MESSAGE_LENGTH]; + + if (ctx == NULL) { + errno = EINVAL; + return -1; + } + + if (sub_addr > MODBUS_MAX_FILE_RECORD_NUMBER) { + if (ctx->debug) { + fprintf(stderr, + "ERROR Too big file record number (%d > %d)\n", + sub_addr, MODBUS_MAX_FILE_RECORD_NUMBER); + } + errno = EMBMDATA; + return -1; + } + + req_length = ctx->backend->build_request_basis(ctx, + MODBUS_FC_WRITE_FILE_RECORD, + 0, 0, req); + + byte_count = nb * 2; + req_length -= 4; + req[req_length++] = 7 + byte_count; // one request header + bytes to write + req[req_length++] = 6; + req[req_length++] = addr >> 8; + req[req_length++] = addr & 0x00ff; + req[req_length++] = sub_addr >> 8; + req[req_length++] = sub_addr & 0x00ff; + req[req_length++] = nb >> 8; + req[req_length++] = nb * 0x00ff; + + for (i = 0; i < nb; i++) { + req[req_length++] = src[i] >> 8; + req[req_length++] = src[i] & 0x00FF; + } + + rc = send_msg(ctx, req, req_length); + if (rc > 0) { + uint8_t rsp[MAX_MESSAGE_LENGTH]; + + rc = _modbus_receive_msg(ctx, rsp, MSG_CONFIRMATION); + if (rc == -1) + return -1; + + rc = check_confirmation(ctx, req, rsp, rc); + } + + return rc; +} + + int modbus_mask_write_register(modbus_t *ctx, int addr, uint16_t and_mask, uint16_t or_mask) { int rc; diff --git a/src/modbus.h b/src/modbus.h index c63f5ceb4..0a4769432 100644 --- a/src/modbus.h +++ b/src/modbus.h @@ -68,6 +68,8 @@ MODBUS_BEGIN_DECLS #define MODBUS_FC_WRITE_MULTIPLE_COILS 0x0F #define MODBUS_FC_WRITE_MULTIPLE_REGISTERS 0x10 #define MODBUS_FC_REPORT_SLAVE_ID 0x11 +#define MODBUS_FC_READ_FILE_RECORD 0x14 +#define MODBUS_FC_WRITE_FILE_RECORD 0x15 #define MODBUS_FC_MASK_WRITE_REGISTER 0x16 #define MODBUS_FC_WRITE_AND_READ_REGISTERS 0x17 @@ -81,6 +83,11 @@ MODBUS_BEGIN_DECLS #define MODBUS_MAX_READ_BITS 2000 #define MODBUS_MAX_WRITE_BITS 1968 +/* Modbus_Application_Protocol_V1_1b.pdf (chapter 6 section 14 page 33) + * Sub-Req. x, Record Number (2 bytes): 1 to 9999 (0x270F) + */ +#define MODBUS_MAX_FILE_RECORD_NUMBER 9999 + /* Modbus_Application_Protocol_V1_1b.pdf (chapter 6 section 3 page 15) * Quantity of Registers to read (2 bytes): 1 to 125 (0x7D) * (chapter 6 section 12 page 31) @@ -211,6 +218,8 @@ MODBUS_API int modbus_write_bit(modbus_t *ctx, int coil_addr, int status); MODBUS_API int modbus_write_register(modbus_t *ctx, int reg_addr, const uint16_t value); MODBUS_API int modbus_write_bits(modbus_t *ctx, int addr, int nb, const uint8_t *data); MODBUS_API int modbus_write_registers(modbus_t *ctx, int addr, int nb, const uint16_t *data); +MODBUS_API int modbus_write_file_record(modbus_t *ctx, int addr, int sub_addr, int nb, const uint16_t *src); +MODBUS_API int modbus_read_file_record(modbus_t *ctx, int addr, int sub_addr, int nb, uint16_t *dest); MODBUS_API int modbus_mask_write_register(modbus_t *ctx, int addr, uint16_t and_mask, uint16_t or_mask); MODBUS_API int modbus_write_and_read_registers(modbus_t *ctx, int write_addr, int write_nb, const uint16_t *src, int read_addr, int read_nb, From 74c2b4ed19aceccbfa4cc21794bfcaf4d75081b3 Mon Sep 17 00:00:00 2001 From: Kamil Wcislo Date: Thu, 27 Apr 2017 18:27:57 +0200 Subject: [PATCH 02/21] Add modbus_file_t and update mapping functions --- src/modbus-private.h | 1 + src/modbus.c | 83 ++++++++++++++++++++++++++++++-- src/modbus.h | 16 +++++- tests/bandwidth-server-many-up.c | 3 +- tests/bandwidth-server-one.c | 3 +- tests/random-test-server.c | 2 +- tests/unit-test-server.c | 3 +- 7 files changed, 102 insertions(+), 9 deletions(-) diff --git a/src/modbus-private.h b/src/modbus-private.h index 198baeffd..1464d71e7 100644 --- a/src/modbus-private.h +++ b/src/modbus-private.h @@ -106,6 +106,7 @@ struct _modbus { void _modbus_init_common(modbus_t *ctx); void _error_print(modbus_t *ctx, const char *context); int _modbus_receive_msg(modbus_t *ctx, uint8_t *msg, msg_type_t msg_type); +void _modbus_free_files(modbus_file_t *files, int nb_files); #ifndef HAVE_STRLCPY size_t strlcpy(char *dest, const char *src, size_t dest_size); diff --git a/src/modbus.c b/src/modbus.c index e8b87ce06..b177b5651 100644 --- a/src/modbus.c +++ b/src/modbus.c @@ -1895,9 +1895,12 @@ modbus_mapping_t* modbus_mapping_new_start_address( unsigned int start_bits, unsigned int nb_bits, unsigned int start_input_bits, unsigned int nb_input_bits, unsigned int start_registers, unsigned int nb_registers, - unsigned int start_input_registers, unsigned int nb_input_registers) + unsigned int start_input_registers, unsigned int nb_input_registers, + unsigned int start_files, unsigned int nb_files, + unsigned int nb_records, unsigned int record_size) { modbus_mapping_t *mb_mapping; + unsigned int i, j; mb_mapping = (modbus_mapping_t *)malloc(sizeof(modbus_mapping_t)); if (mb_mapping == NULL) { @@ -1972,16 +1975,88 @@ modbus_mapping_t* modbus_mapping_new_start_address( nb_input_registers * sizeof(uint16_t)); } + /* 5X */ + mb_mapping->nb_files = nb_files; + mb_mapping->start_files = start_files; + if (nb_files == 0) { + mb_mapping->files = NULL; + } else { + mb_mapping->files = (modbus_file_t *) malloc(nb_files * sizeof(modbus_file_t)); + if (mb_mapping->files == NULL) { + free(mb_mapping->tab_input_registers); + free(mb_mapping->tab_registers); + free(mb_mapping->tab_input_bits); + free(mb_mapping->tab_bits); + free(mb_mapping); + return NULL; + } + memset(mb_mapping->files, 0, + nb_files * sizeof(modbus_file_t)); + + for (i = 0; i < nb_files; i++) { + mb_mapping->files[i].nb_records = nb_records; + mb_mapping->files[i].record_size = record_size; + mb_mapping->files[i].records = (uint16_t **) malloc(nb_records * sizeof(uint16_t *)); + if (mb_mapping->files[i].records == NULL) { + while (i) { + free(mb_mapping->files[i--].records); + } + free(mb_mapping->files); + free(mb_mapping->tab_input_registers); + free(mb_mapping->tab_registers); + free(mb_mapping->tab_input_bits); + free(mb_mapping->tab_bits); + free(mb_mapping); + return NULL; + } + for (j = 0; j < nb_records; j++) { + mb_mapping->files[i].records[j] = (uint16_t *) malloc(record_size * sizeof(uint16_t)); + if (mb_mapping->files[i].records[j] == NULL) { + while (j) { + free(mb_mapping->files[i].records[j--]); + } + while (i) { + free(mb_mapping->files[i--].records); + } + free(mb_mapping->files); + free(mb_mapping->tab_input_registers); + free(mb_mapping->tab_registers); + free(mb_mapping->tab_input_bits); + free(mb_mapping->tab_bits); + free(mb_mapping); + return NULL; + } + } + } + } + return mb_mapping; } modbus_mapping_t* modbus_mapping_new(int nb_bits, int nb_input_bits, - int nb_registers, int nb_input_registers) + int nb_registers, int nb_input_registers, + int nb_files, int nb_records, int record_size) { return modbus_mapping_new_start_address( - 0, nb_bits, 0, nb_input_bits, 0, nb_registers, 0, nb_input_registers); + 0, nb_bits, 0, nb_input_bits, 0, nb_registers, 0, nb_input_registers, + 0, nb_files, nb_records, record_size); } +void _modbus_free_files(modbus_file_t *files, int nb_files) +{ + int i, j; + + if (files == NULL) { + return; + } + + for (i = 0; i < nb_files; i++) { + for (j = 0; j < files[i].nb_records; j++) { + free(files[i].records[j]); + } + free(files[i].records); + } +} /* Frees the 4 arrays */ void modbus_mapping_free(modbus_mapping_t *mb_mapping) { @@ -1993,6 +2068,8 @@ void modbus_mapping_free(modbus_mapping_t *mb_mapping) free(mb_mapping->tab_registers); free(mb_mapping->tab_input_bits); free(mb_mapping->tab_bits); + _modbus_free_files(mb_mapping->files, mb_mapping->nb_files); + free(mb_mapping->files); free(mb_mapping); } diff --git a/src/modbus.h b/src/modbus.h index 0a4769432..a4425b6c7 100644 --- a/src/modbus.h +++ b/src/modbus.h @@ -161,6 +161,12 @@ extern const unsigned int libmodbus_version_micro; typedef struct _modbus modbus_t; +typedef struct _modbus_file_t { + int nb_records; + int record_size; + uint16_t **records; +} modbus_file_t; + typedef struct _modbus_mapping_t { int nb_bits; int start_bits; @@ -170,10 +176,13 @@ typedef struct _modbus_mapping_t { int start_input_registers; int nb_registers; int start_registers; + int nb_files; + int start_files; uint8_t *tab_bits; uint8_t *tab_input_bits; uint16_t *tab_input_registers; uint16_t *tab_registers; + modbus_file_t *files; } modbus_mapping_t; typedef enum @@ -230,10 +239,13 @@ MODBUS_API modbus_mapping_t* modbus_mapping_new_start_address( unsigned int start_bits, unsigned int nb_bits, unsigned int start_input_bits, unsigned int nb_input_bits, unsigned int start_registers, unsigned int nb_registers, - unsigned int start_input_registers, unsigned int nb_input_registers); + unsigned int start_input_registers, unsigned int nb_input_registers, + unsigned int start_files, unsigned int nb_files, + unsigned int nb_records, unsigned int record_size); MODBUS_API modbus_mapping_t* modbus_mapping_new(int nb_bits, int nb_input_bits, - int nb_registers, int nb_input_registers); + int nb_registers, int nb_input_registers, + int nb_files, int nb_records, int record_size); MODBUS_API void modbus_mapping_free(modbus_mapping_t *mb_mapping); MODBUS_API int modbus_send_raw_request(modbus_t *ctx, const uint8_t *raw_req, int raw_req_length); diff --git a/tests/bandwidth-server-many-up.c b/tests/bandwidth-server-many-up.c index 0f00f3a2f..eb86db831 100644 --- a/tests/bandwidth-server-many-up.c +++ b/tests/bandwidth-server-many-up.c @@ -53,7 +53,8 @@ int main(void) ctx = modbus_new_tcp("127.0.0.1", 1502); mb_mapping = modbus_mapping_new(MODBUS_MAX_READ_BITS, 0, - MODBUS_MAX_READ_REGISTERS, 0); + MODBUS_MAX_READ_REGISTERS, 0, + 0, 0, 0); if (mb_mapping == NULL) { fprintf(stderr, "Failed to allocate the mapping: %s\n", modbus_strerror(errno)); diff --git a/tests/bandwidth-server-one.c b/tests/bandwidth-server-one.c index 8971d0763..4519471f8 100644 --- a/tests/bandwidth-server-one.c +++ b/tests/bandwidth-server-one.c @@ -58,7 +58,8 @@ int main(int argc, char *argv[]) } mb_mapping = modbus_mapping_new(MODBUS_MAX_READ_BITS, 0, - MODBUS_MAX_READ_REGISTERS, 0); + MODBUS_MAX_READ_REGISTERS, 0, + 0, 0, 0); if (mb_mapping == NULL) { fprintf(stderr, "Failed to allocate the mapping: %s\n", modbus_strerror(errno)); diff --git a/tests/random-test-server.c b/tests/random-test-server.c index 03bd87056..e5b97a9ca 100644 --- a/tests/random-test-server.c +++ b/tests/random-test-server.c @@ -22,7 +22,7 @@ int main(void) ctx = modbus_new_tcp("127.0.0.1", 1502); /* modbus_set_debug(ctx, TRUE); */ - mb_mapping = modbus_mapping_new(500, 500, 500, 500); + mb_mapping = modbus_mapping_new(500, 500, 500, 500, 0, 0, 0); if (mb_mapping == NULL) { fprintf(stderr, "Failed to allocate the mapping: %s\n", modbus_strerror(errno)); diff --git a/tests/unit-test-server.c b/tests/unit-test-server.c index 7002b10c6..834105d20 100644 --- a/tests/unit-test-server.c +++ b/tests/unit-test-server.c @@ -75,7 +75,8 @@ int main(int argc, char*argv[]) UT_BITS_ADDRESS, UT_BITS_NB, UT_INPUT_BITS_ADDRESS, UT_INPUT_BITS_NB, UT_REGISTERS_ADDRESS, UT_REGISTERS_NB_MAX, - UT_INPUT_REGISTERS_ADDRESS, UT_INPUT_REGISTERS_NB); + UT_INPUT_REGISTERS_ADDRESS, UT_INPUT_REGISTERS_NB, + 0, 0, 0, 0); if (mb_mapping == NULL) { fprintf(stderr, "Failed to allocate the mapping: %s\n", modbus_strerror(errno)); From 268107f2ae78232ff62f8d6e476155a72748d10d Mon Sep 17 00:00:00 2001 From: Kamil Wcislo Date: Thu, 27 Apr 2017 18:28:57 +0200 Subject: [PATCH 03/21] Update compute length for file handling functions --- src/modbus.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/modbus.c b/src/modbus.c index b177b5651..4c50c0b2d 100644 --- a/src/modbus.c +++ b/src/modbus.c @@ -152,6 +152,12 @@ static unsigned int compute_response_length_from_request(modbus_t *ctx, uint8_t /* The response is device specific (the header provides the length) */ return MSG_LENGTH_UNDEFINED; + case MODBUS_FC_READ_FILE_RECORD: + length = 4 + 2 * (req[offset + 7] << 8 | req[offset + 8]); + break; + case MODBUS_FC_WRITE_FILE_RECORD: + length = 9 + 2 * (req[offset + 7] << 8 | req[offset + 8]); + break; case MODBUS_FC_MASK_WRITE_REGISTER: length = 7; break; @@ -260,6 +266,9 @@ static uint8_t compute_meta_length_after_function(int function, } else if (function == MODBUS_FC_WRITE_MULTIPLE_COILS || function == MODBUS_FC_WRITE_MULTIPLE_REGISTERS) { length = 5; + } else if (function == MODBUS_FC_READ_FILE_RECORD || + function == MODBUS_FC_WRITE_FILE_RECORD) { + length = 8; } else if (function == MODBUS_FC_MASK_WRITE_REGISTER) { length = 6; } else if (function == MODBUS_FC_WRITE_AND_READ_REGISTERS) { @@ -280,6 +289,12 @@ static uint8_t compute_meta_length_after_function(int function, case MODBUS_FC_MASK_WRITE_REGISTER: length = 6; break; + case MODBUS_FC_READ_FILE_RECORD: + length = 3; + break; + case MODBUS_FC_WRITE_FILE_RECORD: + length = 8; + break; default: length = 1; } @@ -304,6 +319,10 @@ static int compute_data_length_after_meta(modbus_t *ctx, uint8_t *msg, case MODBUS_FC_WRITE_AND_READ_REGISTERS: length = msg[ctx->backend->header_length + 9]; break; + case MODBUS_FC_READ_FILE_RECORD: + case MODBUS_FC_WRITE_FILE_RECORD: + length = msg[ctx->backend->header_length + 1] - 7; + break; default: length = 0; } @@ -313,6 +332,10 @@ static int compute_data_length_after_meta(modbus_t *ctx, uint8_t *msg, function == MODBUS_FC_REPORT_SLAVE_ID || function == MODBUS_FC_WRITE_AND_READ_REGISTERS) { length = msg[ctx->backend->header_length + 1]; + } else if (function == MODBUS_FC_READ_FILE_RECORD) { + length = msg[ctx->backend->header_length + 2] - 1; + } else if (function == MODBUS_FC_WRITE_FILE_RECORD) { + length = msg[ctx->backend->header_length + 1] - 7; } else { length = 0; } From 6c8ba5f5725335c25025d60e41b56b72a4ad6e40 Mon Sep 17 00:00:00 2001 From: Kamil Wcislo Date: Thu, 27 Apr 2017 18:57:06 +0200 Subject: [PATCH 04/21] Add reply handling for read/write file record functions --- src/modbus.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 76 insertions(+), 2 deletions(-) diff --git a/src/modbus.c b/src/modbus.c index 4c50c0b2d..7f3510e37 100644 --- a/src/modbus.c +++ b/src/modbus.c @@ -626,9 +626,11 @@ static int check_confirmation(modbus_t *ctx, uint8_t *req, case MODBUS_FC_READ_FILE_RECORD: req_nb_value = (req[offset + 7] << 8) + req[offset + 8]; rsp_nb_value = (rsp[offset + 2] - 1) / 2; + break; case MODBUS_FC_WRITE_FILE_RECORD: req_nb_value = (req[offset + 7] << 8) + req[offset + 8]; rsp_nb_value = (rsp[offset + 7] << 8) + rsp[offset + 8]; + break; default: /* 1 Write functions & others */ req_nb_value = rsp_nb_value = 1; @@ -1018,7 +1020,79 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, } } break; + case MODBUS_FC_READ_FILE_RECORD: { + address = (req[offset + 3] << 8) + req[offset + 4]; + int mapping_address = address - mb_mapping->start_files; + uint16_t record_addr = (req[offset + 5] << 8) + req[offset + 6]; + uint16_t record_len = (req[offset + 7] << 8) + req[offset + 8]; + + if (address < 1 || mapping_address < 0 || mapping_address > mb_mapping->nb_files) { + rsp_length = response_exception( + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp, FALSE, + "Illegal file number %d in read_file_record\n", address); + } else if (record_addr > MODBUS_MAX_FILE_RECORD_NUMBER || + record_addr >= mb_mapping->files[mapping_address].nb_records || + record_len >= mb_mapping->files[mapping_address].record_size) { + rsp_length = response_exception( + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp, TRUE, + "Illegal record %d, len 0x%0X (max %d) in read_file_record\n", + record_addr, record_len, mb_mapping->files[mapping_address].nb_records); + } else { + int i; + + rsp_length = ctx->backend->build_response_basis(&sft, rsp); + rsp[rsp_length++] = record_len * 2 + 2; // data len + rsp[rsp_length++] = record_len * 2 + 1; // file resp len + rsp[rsp_length++] = 6; // reference type + for (i = 0; i < record_len; i++) { + rsp[rsp_length++] = mb_mapping->files[mapping_address].records[record_addr][i] >> 8; + rsp[rsp_length++] = mb_mapping->files[mapping_address].records[record_addr][i] & 0xFF; + } + } + } + break; + case MODBUS_FC_WRITE_FILE_RECORD: { + address = (req[offset + 3] << 8) + req[offset + 4]; + int mapping_address = address - mb_mapping->start_files; + uint16_t record_addr = (req[offset + 5] << 8) + req[offset + 6]; + uint16_t record_len = (req[offset + 7] << 8) + req[offset + 8]; + if (address < 1 || mapping_address < 0 || mapping_address > mb_mapping->nb_files) { + rsp_length = response_exception( + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp, FALSE, + "Illegal file number %d in write_file_record\n", address); + } else if (record_addr > MODBUS_MAX_FILE_RECORD_NUMBER || + record_addr >= mb_mapping->files[mapping_address].nb_records || + record_len >= mb_mapping->files[mapping_address].record_size) { + rsp_length = response_exception( + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp, TRUE, + "Illegal record %d, len 0x%0X (max %d) in write_file_record\n", + record_addr, record_len, mb_mapping->files[mapping_address].nb_records); + } else { + int i; + + for (i = 0; i < record_len; i++) { + mb_mapping->files[mapping_address].records[record_addr][i] = (req[offset + 9 + (i *2)] << 8) | + (req[offset + 10 + (i *2)] & 0xFF); + } + + rsp_length = ctx->backend->build_response_basis(&sft, rsp); + rsp[rsp_length++] = record_len * 2 + 7; // data len + rsp[rsp_length++] = 6; // reference type + rsp[rsp_length++] = address >> 8; // file nb + rsp[rsp_length++] = address & 0xFF; + rsp[rsp_length++] = record_addr >> 8; // record nb + rsp[rsp_length++] = record_addr & 0xFF; + rsp[rsp_length++] = record_len >> 8; // record len + rsp[rsp_length++] = record_len & 0xFF; + + for (i = 0; i < record_len; i++) { + rsp[rsp_length++] = mb_mapping->files[mapping_address].records[record_addr][i] >> 8; + rsp[rsp_length++] = mb_mapping->files[mapping_address].records[record_addr][i] & 0xFF; + } + } + } + break; default: rsp_length = response_exception( ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_FUNCTION, rsp, TRUE, @@ -1299,7 +1373,7 @@ int modbus_read_file_record(modbus_t *ctx, int addr, int sub_addr, int nb, uint1 req[req_length++] = sub_addr >> 8; req[req_length++] = sub_addr & 0x00ff; req[req_length++] = nb >> 8; - req[req_length++] = nb * 0x00ff; + req[req_length++] = nb & 0x00ff; rc = send_msg(ctx, req, req_length); if (rc > 0) { @@ -1527,7 +1601,7 @@ int modbus_write_file_record(modbus_t *ctx, int addr, int sub_addr, int nb, cons req[req_length++] = sub_addr >> 8; req[req_length++] = sub_addr & 0x00ff; req[req_length++] = nb >> 8; - req[req_length++] = nb * 0x00ff; + req[req_length++] = nb & 0x00ff; for (i = 0; i < nb; i++) { req[req_length++] = src[i] >> 8; From 843de9ab12f06eb1827eba3a34bc3f95e47b792a Mon Sep 17 00:00:00 2001 From: Kamil Wcislo Date: Mon, 10 Sep 2018 18:23:30 +0200 Subject: [PATCH 05/21] Add documentation for file operations functions --- doc/Makefile.am | 2 ++ doc/modbus_mapping_new.txt | 12 ++++--- doc/modbus_mapping_new_start_address.txt | 17 ++++++---- doc/modbus_read_file_record.txt | 42 ++++++++++++++++++++++++ doc/modbus_write_file_record.txt | 37 +++++++++++++++++++++ 5 files changed, 100 insertions(+), 10 deletions(-) create mode 100644 doc/modbus_read_file_record.txt create mode 100644 doc/modbus_write_file_record.txt diff --git a/doc/Makefile.am b/doc/Makefile.am index 5a52c0409..c7d2c973d 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -25,6 +25,7 @@ TXT3 = \ modbus_read_input_bits.txt \ modbus_read_input_registers.txt \ modbus_read_registers.txt \ + modbus_read_file_record.txt \ modbus_receive_confirmation.txt \ modbus_receive.txt \ modbus_reply_exception.txt \ @@ -60,6 +61,7 @@ TXT3 = \ modbus_write_bits.txt \ modbus_write_bit.txt \ modbus_write_registers.txt \ + modbus_write_file_record.txt \ modbus_write_register.txt TXT7 = libmodbus.txt diff --git a/doc/modbus_mapping_new.txt b/doc/modbus_mapping_new.txt index 71cde1d63..ffdaa93c6 100644 --- a/doc/modbus_mapping_new.txt +++ b/doc/modbus_mapping_new.txt @@ -9,13 +9,15 @@ modbus_mapping_new - allocate four arrays of bits and registers SYNOPSIS -------- -*modbus_mapping_t* modbus_mapping_new(int 'nb_bits', int 'nb_input_bits', int 'nb_registers', int 'nb_input_registers');* +*modbus_mapping_t* modbus_mapping_new(int 'nb_bits', int 'nb_input_bits', + int 'nb_registers', int 'nb_input_registers', + int 'nb_files', int 'nb_records', int 'record_size');* DESCRIPTION ----------- -The *modbus_mapping_new()* function shall allocate four arrays to store bits, -input bits, registers and inputs registers. The pointers are stored in +The *modbus_mapping_new()* function shall allocate six arrays to store bits, +input bits, registers, inputs registers, files and file records. The pointers are stored in modbus_mapping_t structure. All values of the arrays are initialized to zero. This function is equivalent to a call of the @@ -48,7 +50,9 @@ EXAMPLE mb_mapping = modbus_mapping_new(BITS_ADDRESS + BITS_NB, INPUT_BITS_ADDRESS + INPUT_BITS_NB, REGISTERS_ADDRESS + REGISTERS_NB, - INPUT_REGISTERS_ADDRESS + INPUT_REGISTERS_NB); + INPUT_REGISTERS_ADDRESS + INPUT_REGISTERS_NB, + FILES_ADDRESS + FILES_NB, + RECORDS_NB, RECORDS_SIZE); if (mb_mapping == NULL) { fprintf(stderr, "Failed to allocate the mapping: %s\n", modbus_strerror(errno)); diff --git a/doc/modbus_mapping_new_start_address.txt b/doc/modbus_mapping_new_start_address.txt index ec7bfdb35..2612f5dce 100644 --- a/doc/modbus_mapping_new_start_address.txt +++ b/doc/modbus_mapping_new_start_address.txt @@ -12,14 +12,17 @@ SYNOPSIS *modbus_mapping_t* modbus_mapping_new_start_address(int 'start_bits', int 'nb_bits', int 'start_input_bits', int 'nb_input_bits', int 'start_registers', int 'nb_registers', - int 'start_input_registers', int 'nb_input_registers');* + int 'start_input_registers', int 'nb_input_registers', + int 'start_files', int 'nb_files', + int 'nb_records', int 'record_size');* DESCRIPTION ----------- -The _modbus_mapping_new_start_address()_ function shall allocate four arrays to -store bits, input bits, registers and inputs registers. The pointers are stored -in modbus_mapping_t structure. All values of the arrays are initialized to zero. +The _modbus_mapping_new_start_address()_ function shall allocate six arrays to +store bits, input bits, registers, inputs registers, files and file records. +The pointers are stored in modbus_mapping_t structure. All values of the arrays +are initialized to zero. The different starting adresses make it possible to place the mapping at any address in each address space. This way, you can give access to values stored @@ -28,7 +31,7 @@ make available registers from 10000 to 10009, you can use: [source,c] ------------------- -mb_mapping = modbus_mapping_new_start_address(0, 0, 0, 0, 10000, 10, 0, 0); +mb_mapping = modbus_mapping_new_start_address(0, 0, 0, 0, 10000, 10, 0, 0, 0, 0, 0, 0); ------------------- With this code, only 10 registers (`uint16_t`) are allocated. @@ -60,7 +63,9 @@ EXAMPLE mb_mapping = modbus_mapping_new_start_address(BITS_ADDRESS, BITS_NB, INPUT_BITS_ADDRESS, INPUT_BITS_NB, REGISTERS_ADDRESS, REGISTERS_NB, - INPUT_REGISTERS_ADDRESS, INPUT_REGISTERS_NB); + INPUT_REGISTERS_ADDRESS, INPUT_REGISTERS_NB, + FILES_ADDRESS, FILES_NB, + RECORDS_NB, RECORDS_SIZE); if (mb_mapping == NULL) { fprintf(stderr, "Failed to allocate the mapping: %s\n", modbus_strerror(errno)); diff --git a/doc/modbus_read_file_record.txt b/doc/modbus_read_file_record.txt new file mode 100644 index 000000000..10cef3d7f --- /dev/null +++ b/doc/modbus_read_file_record.txt @@ -0,0 +1,42 @@ +modbus_read_file_record(3) +========================== + + +NAME +---- +modbus_read_file_record - read many input registers + + +SYNOPSIS +-------- +*int modbus_read_file_record(modbus_t *'ctx', int 'addr', int 'sub_addr', int 'nb', uint16_t *'dest');* + + +DESCRIPTION +----------- +The *modbus_read_registers()* function shall read the content of the _nb_ +registers stored in a record at _sub_addr_ in a file at _addr_ of the remote device. +The result of reading is stored in _dest_ array as word values (16 bits). + +You must take care to allocate enough memory to store the results in _dest_ +(at least _nb_ * sizeof(uint16_t)). + +The function uses the Modbus function code 0x14 (read file record). + + +RETURN VALUE +------------ +The function shall return the number of read registers if +successful. Otherwise it shall return -1 and set errno. + + +SEE ALSO +-------- +linkmb:modbus_write_file_record[3] + + +AUTHORS +------- +Kamil Wcislo +The libmodbus documentation was written by Stéphane Raimbault + diff --git a/doc/modbus_write_file_record.txt b/doc/modbus_write_file_record.txt new file mode 100644 index 000000000..bb1196d9b --- /dev/null +++ b/doc/modbus_write_file_record.txt @@ -0,0 +1,37 @@ +modbus_write_file_record(3) +=========================== + + +NAME +---- +modbus_write_file_record - read registers from record in a file + + +SYNOPSIS +-------- +*int modbus_write_file_record(modbus_t *'ctx', int 'addr', int 'sub_addr', int 'nb', uint16_t *'src');* + +DESCRIPTION +----------- +The *modbus_write_file_record()* function shall write the content of the _nb_ +holding registers from the array _src_ to record at _sub_addr_ of the file at +address _addr_ of the remote device. + +The function uses the Modbus function code 0x15 (write file record). + +RETURN VALUE +------------ +The function shall return the number of written registers if +successful. Otherwise it shall return -1 and set errno. + + +SEE ALSO +-------- +linkmb:modbus_read_file_record[3] + + +AUTHORS +------- +Kamil Wcislo +The libmodbus documentation was written by Stéphane Raimbault + From b6671bac5e3cc392468913f81b46fb9ae966f2ae Mon Sep 17 00:00:00 2001 From: Kamil Wcislo Date: Mon, 10 Sep 2018 19:55:10 +0200 Subject: [PATCH 06/21] Fix request length allocation for modbus_read_file_record --- src/modbus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modbus.c b/src/modbus.c index 7f3510e37..b1393c3f6 100644 --- a/src/modbus.c +++ b/src/modbus.c @@ -1343,7 +1343,7 @@ int modbus_read_file_record(modbus_t *ctx, int addr, int sub_addr, int nb, uint1 { int rc; int req_length; - uint8_t req[_MIN_REQ_LENGTH]; + uint8_t req[_MIN_REQ_LENGTH + 4]; uint8_t rsp[MAX_MESSAGE_LENGTH]; if (ctx == NULL) { From 195d2348f0fbfffe185e00b9b6db3955eacaf7a7 Mon Sep 17 00:00:00 2001 From: Kamil Wcislo Date: Mon, 10 Sep 2018 19:55:38 +0200 Subject: [PATCH 07/21] Add unit tests for file operations functions --- tests/unit-test-client.c | 18 ++++++++++++++++++ tests/unit-test-server.c | 2 +- tests/unit-test.h.in | 6 ++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/tests/unit-test-client.c b/tests/unit-test-client.c index e95ea69fa..456e2e2c5 100644 --- a/tests/unit-test-client.c +++ b/tests/unit-test-client.c @@ -273,6 +273,24 @@ int main(int argc, char *argv[]) tab_rp_registers[i], UT_INPUT_REGISTERS_TAB[i]); } + /** FILE OPERATIONS **/ + printf("\nTEST FILE OPERATIONS\n"); + rc = modbus_write_file_record(ctx, 0x1, UT_RECORD_ADDRESS, + UT_REGISTERS_NB, UT_REGISTERS_TAB); + printf("1/2 modbus_write_file_record: "); + ASSERT_TRUE(rc == UT_REGISTERS_NB, "FAILED (nb points %d)\n", rc); + + rc = modbus_read_file_record(ctx, 0x1, UT_RECORD_ADDRESS, + UT_REGISTERS_NB, tab_rp_registers); + printf("2/2 modbus_read_file_record: "); + ASSERT_TRUE(rc == UT_REGISTERS_NB, "FAILED (nb points %d)\n", rc); + + for (i=0; i < UT_REGISTERS_NB; i++) { + ASSERT_TRUE(tab_rp_registers[i] == UT_REGISTERS_TAB[i], + "%d: FAILED (%0X != %0X)\n", + i, tab_rp_registers[i], UT_REGISTERS_TAB[i]); + } + /* MASKS */ printf("1/1 Write mask: "); rc = modbus_write_register(ctx, UT_REGISTERS_ADDRESS, 0x12); diff --git a/tests/unit-test-server.c b/tests/unit-test-server.c index 834105d20..0776f6ee8 100644 --- a/tests/unit-test-server.c +++ b/tests/unit-test-server.c @@ -76,7 +76,7 @@ int main(int argc, char*argv[]) UT_INPUT_BITS_ADDRESS, UT_INPUT_BITS_NB, UT_REGISTERS_ADDRESS, UT_REGISTERS_NB_MAX, UT_INPUT_REGISTERS_ADDRESS, UT_INPUT_REGISTERS_NB, - 0, 0, 0, 0); + UT_FILE_ADDRESS, UT_FILE_NB, UT_RECORDS_NB, UT_RECORDS_SIZE); if (mb_mapping == NULL) { fprintf(stderr, "Failed to allocate the mapping: %s\n", modbus_strerror(errno)); diff --git a/tests/unit-test.h.in b/tests/unit-test.h.in index dca826f46..ce5e34190 100644 --- a/tests/unit-test.h.in +++ b/tests/unit-test.h.in @@ -56,6 +56,12 @@ const uint16_t UT_INPUT_REGISTERS_ADDRESS = 0x108; const uint16_t UT_INPUT_REGISTERS_NB = 0x1; const uint16_t UT_INPUT_REGISTERS_TAB[] = { 0x000A }; +const uint16_t UT_FILE_ADDRESS = 0x0; +const uint16_t UT_FILE_NB = 0x2; +const uint16_t UT_RECORDS_NB = 0x3; +const uint16_t UT_RECORDS_SIZE = 0x20; +const uint16_t UT_RECORD_ADDRESS = 0x2; + const float UT_REAL = 123456.00; const uint32_t UT_IREAL_ABCD = 0x0020F147; From 2d51c4c440b040ac5fc9334d408c4d4a3933c55d Mon Sep 17 00:00:00 2001 From: Kamil Wcislo Date: Fri, 14 Sep 2018 09:39:38 +0200 Subject: [PATCH 08/21] doc/modbus_write_file_record.txt: add const to function definition --- doc/modbus_write_file_record.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/modbus_write_file_record.txt b/doc/modbus_write_file_record.txt index bb1196d9b..9b17410c1 100644 --- a/doc/modbus_write_file_record.txt +++ b/doc/modbus_write_file_record.txt @@ -9,7 +9,7 @@ modbus_write_file_record - read registers from record in a file SYNOPSIS -------- -*int modbus_write_file_record(modbus_t *'ctx', int 'addr', int 'sub_addr', int 'nb', uint16_t *'src');* +*int modbus_write_file_record(modbus_t *'ctx', int 'addr', int 'sub_addr', int 'nb', const uint16_t *'src');* DESCRIPTION ----------- From b4069a08381f6fad760f936c163f9e4e0b8d2119 Mon Sep 17 00:00:00 2001 From: Richard Ash Date: Mon, 22 Jun 2020 17:04:03 +0100 Subject: [PATCH 09/21] Add some comments to new code --- src/modbus.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/modbus.c b/src/modbus.c index b1393c3f6..6ea8760cf 100644 --- a/src/modbus.c +++ b/src/modbus.c @@ -1350,7 +1350,7 @@ int modbus_read_file_record(modbus_t *ctx, int addr, int sub_addr, int nb, uint1 errno = EINVAL; return -1; } - + // check if record number is within range if (sub_addr > MODBUS_MAX_FILE_RECORD_NUMBER) { if (ctx->debug) { fprintf(stderr, @@ -1389,7 +1389,8 @@ int modbus_read_file_record(modbus_t *ctx, int addr, int sub_addr, int nb, uint1 return -1; offset = ctx->backend->header_length; - + // if all went well, copy returned data to caller's pointer, converting + // bytes into 16-bit words. for (i = 0; i < rc; i++) { dest[i] = (rsp[offset + 4 + (i << 1)] << 8) | @@ -1577,7 +1578,7 @@ int modbus_write_file_record(modbus_t *ctx, int addr, int sub_addr, int nb, cons errno = EINVAL; return -1; } - + // check record number is valid if (sub_addr > MODBUS_MAX_FILE_RECORD_NUMBER) { if (ctx->debug) { fprintf(stderr, @@ -1602,7 +1603,7 @@ int modbus_write_file_record(modbus_t *ctx, int addr, int sub_addr, int nb, cons req[req_length++] = sub_addr & 0x00ff; req[req_length++] = nb >> 8; req[req_length++] = nb & 0x00ff; - + // pack supplied data into request for (i = 0; i < nb; i++) { req[req_length++] = src[i] >> 8; req[req_length++] = src[i] & 0x00FF; From d4bcb109ff63c40de9cb2ac8f7159e1e2d96f46a Mon Sep 17 00:00:00 2001 From: Richard Ash Date: Tue, 23 Jun 2020 10:11:14 +0100 Subject: [PATCH 10/21] Convert C++ comments to C-style --- src/modbus.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/modbus.c b/src/modbus.c index 6ea8760cf..1f0381fd1 100644 --- a/src/modbus.c +++ b/src/modbus.c @@ -1350,7 +1350,7 @@ int modbus_read_file_record(modbus_t *ctx, int addr, int sub_addr, int nb, uint1 errno = EINVAL; return -1; } - // check if record number is within range + /* check if record number is within range */ if (sub_addr > MODBUS_MAX_FILE_RECORD_NUMBER) { if (ctx->debug) { fprintf(stderr, @@ -1366,7 +1366,7 @@ int modbus_read_file_record(modbus_t *ctx, int addr, int sub_addr, int nb, uint1 0, 0, req); req_length -= 4; - req[req_length++] = 7; // one request, so 7 bytes + req[req_length++] = 7; /* one request, so 7 bytes */ req[req_length++] = 6; req[req_length++] = addr >> 8; req[req_length++] = addr & 0x00ff; @@ -1389,8 +1389,9 @@ int modbus_read_file_record(modbus_t *ctx, int addr, int sub_addr, int nb, uint1 return -1; offset = ctx->backend->header_length; - // if all went well, copy returned data to caller's pointer, converting - // bytes into 16-bit words. + /* if all went well, copy returned data to caller's pointer, converting + * bytes into 16-bit words. + */ for (i = 0; i < rc; i++) { dest[i] = (rsp[offset + 4 + (i << 1)] << 8) | @@ -1578,7 +1579,7 @@ int modbus_write_file_record(modbus_t *ctx, int addr, int sub_addr, int nb, cons errno = EINVAL; return -1; } - // check record number is valid + /* check record number is valid */ if (sub_addr > MODBUS_MAX_FILE_RECORD_NUMBER) { if (ctx->debug) { fprintf(stderr, @@ -1595,7 +1596,7 @@ int modbus_write_file_record(modbus_t *ctx, int addr, int sub_addr, int nb, cons byte_count = nb * 2; req_length -= 4; - req[req_length++] = 7 + byte_count; // one request header + bytes to write + req[req_length++] = 7 + byte_count; /* one request header + bytes to write */ req[req_length++] = 6; req[req_length++] = addr >> 8; req[req_length++] = addr & 0x00ff; @@ -1603,7 +1604,7 @@ int modbus_write_file_record(modbus_t *ctx, int addr, int sub_addr, int nb, cons req[req_length++] = sub_addr & 0x00ff; req[req_length++] = nb >> 8; req[req_length++] = nb & 0x00ff; - // pack supplied data into request + /* pack supplied data into request */ for (i = 0; i < nb; i++) { req[req_length++] = src[i] >> 8; req[req_length++] = src[i] & 0x00FF; From f302db53bb0723ef0a34e15949552fa042c4f6da Mon Sep 17 00:00:00 2001 From: Richard Ash Date: Tue, 23 Jun 2020 10:59:28 +0100 Subject: [PATCH 11/21] Merge in my documentation additions. Combine what I had written from scratch with documentation from @mek-x --- doc/modbus_read_file_record.txt | 37 ++++++++++++++++++++------------ doc/modbus_write_file_record.txt | 36 ++++++++++++++++++++----------- 2 files changed, 47 insertions(+), 26 deletions(-) diff --git a/doc/modbus_read_file_record.txt b/doc/modbus_read_file_record.txt index 10cef3d7f..c14d9751b 100644 --- a/doc/modbus_read_file_record.txt +++ b/doc/modbus_read_file_record.txt @@ -4,39 +4,48 @@ modbus_read_file_record(3) NAME ---- -modbus_read_file_record - read many input registers +modbus_read_file_record - Read records from a file SYNOPSIS -------- -*int modbus_read_file_record(modbus_t *'ctx', int 'addr', int 'sub_addr', int 'nb', uint16_t *'dest');* +*int modbus_read_file_record(modbus_t *'ctx', int 'addr', int 'sub_addr', int 'nb', uint16_t * 'dest');* DESCRIPTION ----------- -The *modbus_read_registers()* function shall read the content of the _nb_ -registers stored in a record at _sub_addr_ in a file at _addr_ of the remote device. -The result of reading is stored in _dest_ array as word values (16 bits). +The *modbus_read_file_record()* function reads _nb_ records from file +number _addr_, starting from record position _sub_addr_ in the file. +The result of reading is stored in _dest_ array as word values (16 bits). You +must take care to allocate enough memory to store the results in _dest_ +- (at least _nb_ * sizeof(uint16_t)). -You must take care to allocate enough memory to store the results in _dest_ -(at least _nb_ * sizeof(uint16_t)). +A file is an array of records, each of 16 bits. -The function uses the Modbus function code 0x14 (read file record). +* A modbus device may have up to 65535 files, addressed 1 to 65535 decimal. +* Each file contains 10000 records, addressed 0000 to 9999 decimal. + +A maximum of 124 records (_nb_) may be retrived in a single request. + +This function uses the Modbus function code 0x14 (Read File Record). Note that +althrough the ModBus Specification allows for multiple non-contiguous reads in +the same file to be made in a single request, this function only supports a +single contiguous read request. RETURN VALUE ------------ -The function shall return the number of read registers if -successful. Otherwise it shall return -1 and set errno. +The function shall return the number of records read (i.e. the value of _nb_) if successful. +Otherwise it shall return -1 and set errno. SEE ALSO -------- -linkmb:modbus_write_file_record[3] +linkmb:modbus_read_register[3] +linkmb:modbus_write_file_register[3] AUTHORS ------- -Kamil Wcislo -The libmodbus documentation was written by Stéphane Raimbault - +File record support by Kamil Wcislo , documentation +extended by Richard Ash at EA Technology. diff --git a/doc/modbus_write_file_record.txt b/doc/modbus_write_file_record.txt index 9b17410c1..66b715a65 100644 --- a/doc/modbus_write_file_record.txt +++ b/doc/modbus_write_file_record.txt @@ -4,34 +4,46 @@ modbus_write_file_record(3) NAME ---- -modbus_write_file_record - read registers from record in a file +modbus_write_file_record - Write records to a file SYNOPSIS -------- -*int modbus_write_file_record(modbus_t *'ctx', int 'addr', int 'sub_addr', int 'nb', const uint16_t *'src');* +*int modbus_write_file_record(modbus_t *'ctx', int 'addr', int 'sub_addr', int 'nb', const uint16_t * 'src');* + DESCRIPTION ----------- -The *modbus_write_file_record()* function shall write the content of the _nb_ -holding registers from the array _src_ to record at _sub_addr_ of the file at -address _addr_ of the remote device. +The *modbus_write_file_record()* function writes the content of the _nb_ +records from the _src_ array to file number _addr_, starting from position +_sub_addr_ in the file. + +A file is an array of records, each of 16 bits. + +* A modbus device may have up to 65535 files, addressed 1 to 65535 decimal. +* Each file contains 10000 records, addressed 0000 to 9999 decimal. + +A maximum of 124 records (_nb_) may be written in a single request. + +This function uses the Modbus function code 0x15 (Write File Record). Note that +althrough the ModBus Specification allows for multiple non-contiguous writes in +the same file to be made in a single request, this function only supports a +single contiguous write request. -The function uses the Modbus function code 0x15 (write file record). RETURN VALUE ------------ -The function shall return the number of written registers if -successful. Otherwise it shall return -1 and set errno. +The function shall return the number of records written (i.e. the value of _nb_) if successful. +Otherwise it shall return -1 and set errno. SEE ALSO -------- -linkmb:modbus_read_file_record[3] +linkmb:modbus_write_register[3] +linkmb:modbus_read_file_register[3] AUTHORS ------- -Kamil Wcislo -The libmodbus documentation was written by Stéphane Raimbault - +The libmodbus file support was added by Kamil Wcislo and +the documentation extended by Richard Ash at EA Technology. From fa874c09e9c1c542ea183caf0177be47aaddb82f Mon Sep 17 00:00:00 2001 From: Richard Ash Date: Tue, 23 Jun 2020 11:46:37 +0100 Subject: [PATCH 12/21] Expand the unit tests for files This should test that successive writes can modify adjacent file areas, and that one read can read back both. But it fails at present. --- tests/unit-test-client.c | 37 ++++++++++++++++++++++++++++++------- tests/unit-test-server.c | 1 + tests/unit-test.h.in | 9 ++++++--- 3 files changed, 37 insertions(+), 10 deletions(-) diff --git a/tests/unit-test-client.c b/tests/unit-test-client.c index 456e2e2c5..89b7f60f7 100644 --- a/tests/unit-test-client.c +++ b/tests/unit-test-client.c @@ -49,6 +49,7 @@ int main(int argc, char *argv[]) const int NB_REPORT_SLAVE_ID = 10; uint8_t *tab_rp_bits = NULL; uint16_t *tab_rp_registers = NULL; + uint16_t *tab_file_records = NULL; /* table to hold file records */ uint16_t *tab_rp_registers_bad = NULL; modbus_t *ctx = NULL; int i; @@ -275,20 +276,42 @@ int main(int argc, char *argv[]) /** FILE OPERATIONS **/ printf("\nTEST FILE OPERATIONS\n"); - rc = modbus_write_file_record(ctx, 0x1, UT_RECORD_ADDRESS, + /* Write three records to file 1 on server, starting at record 2 */ + rc = modbus_write_file_record(ctx, UT_FILE, UT_RECORD_ADDRESS, UT_REGISTERS_NB, UT_REGISTERS_TAB); printf("1/2 modbus_write_file_record: "); ASSERT_TRUE(rc == UT_REGISTERS_NB, "FAILED (nb points %d)\n", rc); + /* Write four more records to file 1 on server, starting at record 5, + * i.e. after previous records */ + nb_points = UT_RECORD_ADDRESS + UT_REGISTERS_NB; + rc = modbus_write_file_record(ctx, UT_FILE, nb_points, + UT_FILE_RECORD_NB, UT_FILE_TAB); + /* Read back all 7 records and make sure the values are correct. + * Allocate and initialize the memory to store the registers as we need more. + */ + nb_points = UT_REGISTERS_NB + UT_FILE_RECORD_NB; + + tab_file_records = (uint16_t *) malloc(nb_points * sizeof(uint16_t)); + memset(tab_file_records, 0, nb_points * sizeof(uint16_t)); - rc = modbus_read_file_record(ctx, 0x1, UT_RECORD_ADDRESS, - UT_REGISTERS_NB, tab_rp_registers); + rc = modbus_read_file_record(ctx, UT_FILE, UT_RECORD_ADDRESS, + nb_points, tab_file_records); printf("2/2 modbus_read_file_record: "); - ASSERT_TRUE(rc == UT_REGISTERS_NB, "FAILED (nb points %d)\n", rc); + ASSERT_TRUE(rc == nb_points, "FAILED (nb points %d)\n", rc); - for (i=0; i < UT_REGISTERS_NB; i++) { - ASSERT_TRUE(tab_rp_registers[i] == UT_REGISTERS_TAB[i], + for (i=0; i < nb_points; i++) { + printf("%d: 0x%0X\n", i, tab_file_records[i]); + if (i < UT_REGISTERS_NB) { /* first three in UT_REGISTERS_TAB */ + ASSERT_TRUE(tab_file_records[i] == UT_REGISTERS_TAB[i], + "%d: FAILED (%0X != %0X)\n", + i, tab_file_records[i], UT_REGISTERS_TAB[i]); + } else { /* Rest in UT_FILE_TAB */ + /* calculate the index in this table */ + rc = i - UT_REGISTERS_NB; + ASSERT_TRUE(tab_file_records[i] == UT_FILE_TAB[rc], "%d: FAILED (%0X != %0X)\n", - i, tab_rp_registers[i], UT_REGISTERS_TAB[i]); + i, tab_file_records[i], UT_FILE_TAB[rc]); + } } /* MASKS */ diff --git a/tests/unit-test-server.c b/tests/unit-test-server.c index 0776f6ee8..b2bd1d5a7 100644 --- a/tests/unit-test-server.c +++ b/tests/unit-test-server.c @@ -76,6 +76,7 @@ int main(int argc, char*argv[]) UT_INPUT_BITS_ADDRESS, UT_INPUT_BITS_NB, UT_REGISTERS_ADDRESS, UT_REGISTERS_NB_MAX, UT_INPUT_REGISTERS_ADDRESS, UT_INPUT_REGISTERS_NB, + /* 2 files, each 10 records */ UT_FILE_ADDRESS, UT_FILE_NB, UT_RECORDS_NB, UT_RECORDS_SIZE); if (mb_mapping == NULL) { fprintf(stderr, "Failed to allocate the mapping: %s\n", diff --git a/tests/unit-test.h.in b/tests/unit-test.h.in index ce5e34190..dedaa3abf 100644 --- a/tests/unit-test.h.in +++ b/tests/unit-test.h.in @@ -37,6 +37,9 @@ const uint16_t UT_REGISTERS_ADDRESS = 0x160; const uint16_t UT_REGISTERS_NB = 0x3; const uint16_t UT_REGISTERS_NB_MAX = 0x20; const uint16_t UT_REGISTERS_TAB[] = { 0x022B, 0x0001, 0x0064 }; +const uint16_t UT_FILE_TAB[] = { 0x021B, 0x0002, 0x0063, 0x0042 }; +const uint16_t UT_FILE_RECORD_NB = 0x4; +const uint16_t UT_FILE = 0x1; /* Raise a manual exception when this address is used for the first byte */ const uint16_t UT_REGISTERS_ADDRESS_SPECIAL = 0x170; @@ -56,9 +59,9 @@ const uint16_t UT_INPUT_REGISTERS_ADDRESS = 0x108; const uint16_t UT_INPUT_REGISTERS_NB = 0x1; const uint16_t UT_INPUT_REGISTERS_TAB[] = { 0x000A }; -const uint16_t UT_FILE_ADDRESS = 0x0; -const uint16_t UT_FILE_NB = 0x2; -const uint16_t UT_RECORDS_NB = 0x3; +const uint16_t UT_FILE_ADDRESS = 0x0; /* Number of first file on server */ +const uint16_t UT_FILE_NB = 0x2; /* How many files available on server */ +const uint16_t UT_RECORDS_NB = 0xF; /* How many records per file */ const uint16_t UT_RECORDS_SIZE = 0x20; const uint16_t UT_RECORD_ADDRESS = 0x2; From f51d7a2ee5ed7ca4aa776432b173dd6f10990124 Mon Sep 17 00:00:00 2001 From: Richard Ash Date: Tue, 23 Jun 2020 12:21:52 +0100 Subject: [PATCH 13/21] Fix the server side of file support There seems to have been a miss-understanding of the documentation leading to the implementation being more complex than necessary, and incorrect. Fixed, it now works the same way as #407 does, and the unit tests pass again. --- src/modbus.c | 60 ++++++++++---------------------- src/modbus.h | 9 +++-- tests/bandwidth-server-many-up.c | 2 +- tests/bandwidth-server-one.c | 2 +- tests/random-test-server.c | 2 +- tests/unit-test-server.c | 2 +- 6 files changed, 27 insertions(+), 50 deletions(-) diff --git a/src/modbus.c b/src/modbus.c index 1f0381fd1..5a05bfbc4 100644 --- a/src/modbus.c +++ b/src/modbus.c @@ -1031,8 +1031,8 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp, FALSE, "Illegal file number %d in read_file_record\n", address); } else if (record_addr > MODBUS_MAX_FILE_RECORD_NUMBER || - record_addr >= mb_mapping->files[mapping_address].nb_records || - record_len >= mb_mapping->files[mapping_address].record_size) { + /* index of last record (zero-based) is beyond end of file */ + (record_addr + record_len) > mb_mapping->files[mapping_address].nb_records) { rsp_length = response_exception( ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp, TRUE, "Illegal record %d, len 0x%0X (max %d) in read_file_record\n", @@ -1045,8 +1045,8 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, rsp[rsp_length++] = record_len * 2 + 1; // file resp len rsp[rsp_length++] = 6; // reference type for (i = 0; i < record_len; i++) { - rsp[rsp_length++] = mb_mapping->files[mapping_address].records[record_addr][i] >> 8; - rsp[rsp_length++] = mb_mapping->files[mapping_address].records[record_addr][i] & 0xFF; + rsp[rsp_length++] = mb_mapping->files[mapping_address].records[record_addr + i] >> 8; + rsp[rsp_length++] = mb_mapping->files[mapping_address].records[record_addr + i] & 0xFF; } } } @@ -1063,7 +1063,8 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, "Illegal file number %d in write_file_record\n", address); } else if (record_addr > MODBUS_MAX_FILE_RECORD_NUMBER || record_addr >= mb_mapping->files[mapping_address].nb_records || - record_len >= mb_mapping->files[mapping_address].record_size) { + /* index of last record (zero-based) is beyond end of file */ + (record_addr + record_len) > mb_mapping->files[mapping_address].nb_records) { rsp_length = response_exception( ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp, TRUE, "Illegal record %d, len 0x%0X (max %d) in write_file_record\n", @@ -1072,7 +1073,7 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, int i; for (i = 0; i < record_len; i++) { - mb_mapping->files[mapping_address].records[record_addr][i] = (req[offset + 9 + (i *2)] << 8) | + mb_mapping->files[mapping_address].records[record_addr + i] = (req[offset + 9 + (i *2)] << 8) | (req[offset + 10 + (i *2)] & 0xFF); } @@ -1087,8 +1088,8 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, rsp[rsp_length++] = record_len & 0xFF; for (i = 0; i < record_len; i++) { - rsp[rsp_length++] = mb_mapping->files[mapping_address].records[record_addr][i] >> 8; - rsp[rsp_length++] = mb_mapping->files[mapping_address].records[record_addr][i] & 0xFF; + rsp[rsp_length++] = mb_mapping->files[mapping_address].records[record_addr + i] >> 8; + rsp[rsp_length++] = mb_mapping->files[mapping_address].records[record_addr + i] & 0xFF; } } } @@ -1996,10 +1997,10 @@ modbus_mapping_t* modbus_mapping_new_start_address( unsigned int start_registers, unsigned int nb_registers, unsigned int start_input_registers, unsigned int nb_input_registers, unsigned int start_files, unsigned int nb_files, - unsigned int nb_records, unsigned int record_size) + unsigned int nb_records) { modbus_mapping_t *mb_mapping; - unsigned int i, j; + unsigned int i; mb_mapping = (modbus_mapping_t *)malloc(sizeof(modbus_mapping_t)); if (mb_mapping == NULL) { @@ -2092,14 +2093,10 @@ modbus_mapping_t* modbus_mapping_new_start_address( memset(mb_mapping->files, 0, nb_files * sizeof(modbus_file_t)); - for (i = 0; i < nb_files; i++) { + for (i = 0; i < nb_files; i++) { /* for each file */ mb_mapping->files[i].nb_records = nb_records; - mb_mapping->files[i].record_size = record_size; - mb_mapping->files[i].records = (uint16_t **) malloc(nb_records * sizeof(uint16_t *)); + mb_mapping->files[i].records = (uint16_t *) malloc(nb_records * sizeof(uint16_t)); if (mb_mapping->files[i].records == NULL) { - while (i) { - free(mb_mapping->files[i--].records); - } free(mb_mapping->files); free(mb_mapping->tab_input_registers); free(mb_mapping->tab_registers); @@ -2108,24 +2105,8 @@ modbus_mapping_t* modbus_mapping_new_start_address( free(mb_mapping); return NULL; } - for (j = 0; j < nb_records; j++) { - mb_mapping->files[i].records[j] = (uint16_t *) malloc(record_size * sizeof(uint16_t)); - if (mb_mapping->files[i].records[j] == NULL) { - while (j) { - free(mb_mapping->files[i].records[j--]); - } - while (i) { - free(mb_mapping->files[i--].records); - } - free(mb_mapping->files); - free(mb_mapping->tab_input_registers); - free(mb_mapping->tab_registers); - free(mb_mapping->tab_input_bits); - free(mb_mapping->tab_bits); - free(mb_mapping); - return NULL; - } - } + /* zero the file records */ + memset(mb_mapping->files[i].records, 0, nb_records * sizeof(uint16_t)); } } @@ -2134,25 +2115,22 @@ modbus_mapping_t* modbus_mapping_new_start_address( modbus_mapping_t* modbus_mapping_new(int nb_bits, int nb_input_bits, int nb_registers, int nb_input_registers, - int nb_files, int nb_records, int record_size) + int nb_files, int nb_records) { return modbus_mapping_new_start_address( 0, nb_bits, 0, nb_input_bits, 0, nb_registers, 0, nb_input_registers, - 0, nb_files, nb_records, record_size); + 0, nb_files, nb_records); } void _modbus_free_files(modbus_file_t *files, int nb_files) { - int i, j; + int i; if (files == NULL) { return; } - + /* Free the file contents records */ for (i = 0; i < nb_files; i++) { - for (j = 0; j < files[i].nb_records; j++) { - free(files[i].records[j]); - } free(files[i].records); } } diff --git a/src/modbus.h b/src/modbus.h index a4425b6c7..b5acc3204 100644 --- a/src/modbus.h +++ b/src/modbus.h @@ -162,9 +162,8 @@ extern const unsigned int libmodbus_version_micro; typedef struct _modbus modbus_t; typedef struct _modbus_file_t { - int nb_records; - int record_size; - uint16_t **records; + int nb_records; /* Maximum number of records in file */ + uint16_t *records; /* Pointer to memory for record storage */ } modbus_file_t; typedef struct _modbus_mapping_t { @@ -241,11 +240,11 @@ MODBUS_API modbus_mapping_t* modbus_mapping_new_start_address( unsigned int start_registers, unsigned int nb_registers, unsigned int start_input_registers, unsigned int nb_input_registers, unsigned int start_files, unsigned int nb_files, - unsigned int nb_records, unsigned int record_size); + unsigned int nb_records); MODBUS_API modbus_mapping_t* modbus_mapping_new(int nb_bits, int nb_input_bits, int nb_registers, int nb_input_registers, - int nb_files, int nb_records, int record_size); + int nb_files, int nb_records); MODBUS_API void modbus_mapping_free(modbus_mapping_t *mb_mapping); MODBUS_API int modbus_send_raw_request(modbus_t *ctx, const uint8_t *raw_req, int raw_req_length); diff --git a/tests/bandwidth-server-many-up.c b/tests/bandwidth-server-many-up.c index eb86db831..51a699555 100644 --- a/tests/bandwidth-server-many-up.c +++ b/tests/bandwidth-server-many-up.c @@ -54,7 +54,7 @@ int main(void) mb_mapping = modbus_mapping_new(MODBUS_MAX_READ_BITS, 0, MODBUS_MAX_READ_REGISTERS, 0, - 0, 0, 0); + 0, 0); if (mb_mapping == NULL) { fprintf(stderr, "Failed to allocate the mapping: %s\n", modbus_strerror(errno)); diff --git a/tests/bandwidth-server-one.c b/tests/bandwidth-server-one.c index 4519471f8..3ce3087ab 100644 --- a/tests/bandwidth-server-one.c +++ b/tests/bandwidth-server-one.c @@ -59,7 +59,7 @@ int main(int argc, char *argv[]) mb_mapping = modbus_mapping_new(MODBUS_MAX_READ_BITS, 0, MODBUS_MAX_READ_REGISTERS, 0, - 0, 0, 0); + 0, 0); if (mb_mapping == NULL) { fprintf(stderr, "Failed to allocate the mapping: %s\n", modbus_strerror(errno)); diff --git a/tests/random-test-server.c b/tests/random-test-server.c index e5b97a9ca..2621abe69 100644 --- a/tests/random-test-server.c +++ b/tests/random-test-server.c @@ -22,7 +22,7 @@ int main(void) ctx = modbus_new_tcp("127.0.0.1", 1502); /* modbus_set_debug(ctx, TRUE); */ - mb_mapping = modbus_mapping_new(500, 500, 500, 500, 0, 0, 0); + mb_mapping = modbus_mapping_new(500, 500, 500, 500, 0, 0); if (mb_mapping == NULL) { fprintf(stderr, "Failed to allocate the mapping: %s\n", modbus_strerror(errno)); diff --git a/tests/unit-test-server.c b/tests/unit-test-server.c index b2bd1d5a7..b4e8d7125 100644 --- a/tests/unit-test-server.c +++ b/tests/unit-test-server.c @@ -77,7 +77,7 @@ int main(int argc, char*argv[]) UT_REGISTERS_ADDRESS, UT_REGISTERS_NB_MAX, UT_INPUT_REGISTERS_ADDRESS, UT_INPUT_REGISTERS_NB, /* 2 files, each 10 records */ - UT_FILE_ADDRESS, UT_FILE_NB, UT_RECORDS_NB, UT_RECORDS_SIZE); + UT_FILE_ADDRESS, UT_FILE_NB, UT_RECORDS_NB); if (mb_mapping == NULL) { fprintf(stderr, "Failed to allocate the mapping: %s\n", modbus_strerror(errno)); From 9e51b4d492be7e1bb4e0eebf8e12709dc1206381 Mon Sep 17 00:00:00 2001 From: Richard Ash Date: Tue, 23 Jun 2020 17:19:46 +0100 Subject: [PATCH 14/21] Adding to the unit tests to show files are separate. This showed up a bug in how files are indexed (because the specification is clear that file numbers start from 1) which is fixed here. --- src/modbus.c | 7 +++++-- tests/unit-test-client.c | 45 +++++++++++++++++++++++++++++++++++++--- tests/unit-test.h.in | 3 +-- 3 files changed, 48 insertions(+), 7 deletions(-) diff --git a/src/modbus.c b/src/modbus.c index 5a05bfbc4..40d73d3b9 100644 --- a/src/modbus.c +++ b/src/modbus.c @@ -1035,7 +1035,7 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, (record_addr + record_len) > mb_mapping->files[mapping_address].nb_records) { rsp_length = response_exception( ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp, TRUE, - "Illegal record %d, len 0x%0X (max %d) in read_file_record\n", + "Illegal request for record %d, len %d (in file len %d) in read_file_record\n", record_addr, record_len, mb_mapping->files[mapping_address].nb_records); } else { int i; @@ -2119,7 +2119,10 @@ modbus_mapping_t* modbus_mapping_new(int nb_bits, int nb_input_bits, { return modbus_mapping_new_start_address( 0, nb_bits, 0, nb_input_bits, 0, nb_registers, 0, nb_input_registers, - 0, nb_files, nb_records); + /* Files are numbered from 1 in the specification, so 1 is the lowest + * start allowed. + */ + 1, nb_files, nb_records); } void _modbus_free_files(modbus_file_t *files, int nb_files) diff --git a/tests/unit-test-client.c b/tests/unit-test-client.c index 89b7f60f7..b75955369 100644 --- a/tests/unit-test-client.c +++ b/tests/unit-test-client.c @@ -276,16 +276,37 @@ int main(int argc, char *argv[]) /** FILE OPERATIONS **/ printf("\nTEST FILE OPERATIONS\n"); + /* Read the whole of the all the files on server to prove they are empty */ + for (value = UT_FILE_ADDRESS; value < (UT_FILE_ADDRESS + UT_FILE_NB); value++) { + tab_file_records = (uint16_t *) malloc(UT_RECORDS_NB * sizeof(uint16_t)); + memset(tab_file_records, 0, UT_RECORDS_NB * sizeof(uint16_t)); + + rc = modbus_read_file_record(ctx, value, 0, + UT_RECORDS_NB, tab_file_records); + printf("%d/6 modbus_read_file_record: ", value); + ASSERT_TRUE(rc == UT_RECORDS_NB, "FAILED (nb points %d)\n", rc); + + for (i=0; i < UT_RECORDS_NB; i++) { + ASSERT_TRUE(tab_file_records[i] == 0x0, + "%d: FAILED (%0X != %0X)\n", + i, tab_file_records[i], 0x0); + } + free(tab_file_records); + } + /* Write three records to file 1 on server, starting at record 2 */ rc = modbus_write_file_record(ctx, UT_FILE, UT_RECORD_ADDRESS, UT_REGISTERS_NB, UT_REGISTERS_TAB); - printf("1/2 modbus_write_file_record: "); + printf("3/6 modbus_write_file_record: "); ASSERT_TRUE(rc == UT_REGISTERS_NB, "FAILED (nb points %d)\n", rc); /* Write four more records to file 1 on server, starting at record 5, * i.e. after previous records */ nb_points = UT_RECORD_ADDRESS + UT_REGISTERS_NB; rc = modbus_write_file_record(ctx, UT_FILE, nb_points, UT_FILE_RECORD_NB, UT_FILE_TAB); + printf("4/6 modbus_write_file_record: "); + ASSERT_TRUE(rc == UT_FILE_RECORD_NB, "FAILED (nb points %d)\n", rc); + /* Read back all 7 records and make sure the values are correct. * Allocate and initialize the memory to store the registers as we need more. */ @@ -296,11 +317,10 @@ int main(int argc, char *argv[]) rc = modbus_read_file_record(ctx, UT_FILE, UT_RECORD_ADDRESS, nb_points, tab_file_records); - printf("2/2 modbus_read_file_record: "); + printf("5/6 modbus_read_file_record: "); ASSERT_TRUE(rc == nb_points, "FAILED (nb points %d)\n", rc); for (i=0; i < nb_points; i++) { - printf("%d: 0x%0X\n", i, tab_file_records[i]); if (i < UT_REGISTERS_NB) { /* first three in UT_REGISTERS_TAB */ ASSERT_TRUE(tab_file_records[i] == UT_REGISTERS_TAB[i], "%d: FAILED (%0X != %0X)\n", @@ -313,6 +333,25 @@ int main(int argc, char *argv[]) i, tab_file_records[i], UT_FILE_TAB[rc]); } } + free(tab_file_records); + + /* Read the whole of the all the files except the first to prove they are still empty */ + for (value = UT_FILE_ADDRESS + 1; value < (UT_FILE_ADDRESS + UT_FILE_NB); value++) { + tab_file_records = (uint16_t *) malloc(UT_RECORDS_NB * sizeof(uint16_t)); + memset(tab_file_records, 0, UT_RECORDS_NB * sizeof(uint16_t)); + + rc = modbus_read_file_record(ctx, value, 0, + UT_RECORDS_NB, tab_file_records); + printf("%d/6 modbus_read_file_record: ", value + 4); + ASSERT_TRUE(rc == UT_RECORDS_NB, "FAILED (nb points %d)\n", rc); + + for (i=0; i < UT_RECORDS_NB; i++) { + ASSERT_TRUE(tab_file_records[i] == 0x0, + "%d: FAILED (%0X != %0X)\n", + i, tab_file_records[i], 0x0); + } + free(tab_file_records); + } /* MASKS */ printf("1/1 Write mask: "); diff --git a/tests/unit-test.h.in b/tests/unit-test.h.in index dedaa3abf..47369dfee 100644 --- a/tests/unit-test.h.in +++ b/tests/unit-test.h.in @@ -59,10 +59,9 @@ const uint16_t UT_INPUT_REGISTERS_ADDRESS = 0x108; const uint16_t UT_INPUT_REGISTERS_NB = 0x1; const uint16_t UT_INPUT_REGISTERS_TAB[] = { 0x000A }; -const uint16_t UT_FILE_ADDRESS = 0x0; /* Number of first file on server */ +const uint16_t UT_FILE_ADDRESS = 0x1; /* Number of first file on server. Files are numbered from 1 in the spec. */ const uint16_t UT_FILE_NB = 0x2; /* How many files available on server */ const uint16_t UT_RECORDS_NB = 0xF; /* How many records per file */ -const uint16_t UT_RECORDS_SIZE = 0x20; const uint16_t UT_RECORD_ADDRESS = 0x2; const float UT_REAL = 123456.00; From b47e70ae54913062b8287b4e1e4aeabc8f7e595c Mon Sep 17 00:00:00 2001 From: Richard Ash Date: Tue, 23 Jun 2020 17:26:10 +0100 Subject: [PATCH 15/21] Remove record size parameter from docs Minimum change to documentation to match change to the code. --- doc/modbus_mapping_new.txt | 6 +++--- doc/modbus_mapping_new_start_address.txt | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/doc/modbus_mapping_new.txt b/doc/modbus_mapping_new.txt index ffdaa93c6..17e31edff 100644 --- a/doc/modbus_mapping_new.txt +++ b/doc/modbus_mapping_new.txt @@ -11,7 +11,7 @@ SYNOPSIS -------- *modbus_mapping_t* modbus_mapping_new(int 'nb_bits', int 'nb_input_bits', int 'nb_registers', int 'nb_input_registers', - int 'nb_files', int 'nb_records', int 'record_size');* + int 'nb_files', int 'nb_records'); DESCRIPTION @@ -22,7 +22,7 @@ modbus_mapping_t structure. All values of the arrays are initialized to zero. This function is equivalent to a call of the linkmb:modbus_mapping_new_start_address[3] function with all start addresses to -`0`. +`0` (except the file start address, which is 1, the smallest value permitted). If it isn't necessary to allocate an array for a specific type of data, you can pass the zero value in argument, the associated pointer will be NULL. @@ -52,7 +52,7 @@ mb_mapping = modbus_mapping_new(BITS_ADDRESS + BITS_NB, REGISTERS_ADDRESS + REGISTERS_NB, INPUT_REGISTERS_ADDRESS + INPUT_REGISTERS_NB, FILES_ADDRESS + FILES_NB, - RECORDS_NB, RECORDS_SIZE); + RECORDS_NB); if (mb_mapping == NULL) { fprintf(stderr, "Failed to allocate the mapping: %s\n", modbus_strerror(errno)); diff --git a/doc/modbus_mapping_new_start_address.txt b/doc/modbus_mapping_new_start_address.txt index 2612f5dce..4db83a47e 100644 --- a/doc/modbus_mapping_new_start_address.txt +++ b/doc/modbus_mapping_new_start_address.txt @@ -14,7 +14,7 @@ SYNOPSIS int 'start_registers', int 'nb_registers', int 'start_input_registers', int 'nb_input_registers', int 'start_files', int 'nb_files', - int 'nb_records', int 'record_size');* + int 'nb_records'); DESCRIPTION @@ -65,7 +65,7 @@ mb_mapping = modbus_mapping_new_start_address(BITS_ADDRESS, BITS_NB, REGISTERS_ADDRESS, REGISTERS_NB, INPUT_REGISTERS_ADDRESS, INPUT_REGISTERS_NB, FILES_ADDRESS, FILES_NB, - RECORDS_NB, RECORDS_SIZE); + RECORDS_NB); if (mb_mapping == NULL) { fprintf(stderr, "Failed to allocate the mapping: %s\n", modbus_strerror(errno)); From 741a664f2f51a47cd7bc959cea09be30989922f7 Mon Sep 17 00:00:00 2001 From: Richard Ash Date: Tue, 23 Jun 2020 18:02:47 +0100 Subject: [PATCH 16/21] Add unit tests for error handling in file records Check that the correct errors occur for likely out-of-range conditions. --- tests/unit-test-client.c | 57 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/tests/unit-test-client.c b/tests/unit-test-client.c index b75955369..b19bbac37 100644 --- a/tests/unit-test-client.c +++ b/tests/unit-test-client.c @@ -352,6 +352,63 @@ int main(int argc, char *argv[]) } free(tab_file_records); } + printf("Intentionally making bad requests to test server error handling ...\n"); + /* Write three records to file 1 on server, starting at record 16 (beyond file end) */ + errno = 0; + rc = modbus_write_file_record(ctx, UT_FILE, UT_RECORDS_NB + 1, + UT_REGISTERS_NB, UT_REGISTERS_TAB); + printf("1/4 modbus_write_file_record: rc %d, err %s, ", rc, modbus_strerror(errno)); + ASSERT_TRUE(((rc == -1) && (errno == EMBXILADD)), "FAILED (incorrect error handling)\n"); + /* Write three records to file 1 on server, starting at record 14 + * (before file end, but write goes over) */ + errno = 0; + rc = modbus_write_file_record(ctx, UT_FILE, UT_RECORDS_NB - 1, + UT_REGISTERS_NB, UT_REGISTERS_TAB); + printf("2/4 modbus_write_file_record: rc %d, err %s, ", rc, modbus_strerror(errno)); + ASSERT_TRUE(((rc == -1) && (errno == EMBXILADD)), "FAILED (incorrect error handling)\n"); + /* Write three records to file 3 on server (which does not exist), + * starting at record 1 (which is OK) */ + errno = 0; + rc = modbus_write_file_record(ctx, UT_FILE + 2, 1, + UT_REGISTERS_NB, UT_REGISTERS_TAB); + printf("3/4 modbus_write_file_record: rc %d, err %s, ", rc, modbus_strerror(errno)); + ASSERT_TRUE(((rc == -1) && (errno == EMBXILADD)), "FAILED (incorrect error handling)\n"); + /* Write three records to file 0 on server (which is not allowed), + * starting at record 1 (which is OK) */ + errno = 0; + rc = modbus_write_file_record(ctx, 0, 1, + UT_REGISTERS_NB, UT_REGISTERS_TAB); + printf("4/4 modbus_write_file_record: rc %d, err %s, ", rc, modbus_strerror(errno)); + ASSERT_TRUE(((rc == -1) && (errno == EMBXILADD)), "FAILED (incorrect error handling)\n"); + + tab_file_records = (uint16_t *) malloc(UT_REGISTERS_NB * sizeof(uint16_t)); + /* Read three records from file 1 on server, starting at record 16 (beyond file end) */ + errno = 0; + rc = modbus_read_file_record(ctx, UT_FILE, UT_RECORDS_NB + 1, + UT_REGISTERS_NB, tab_file_records); + printf("1/4 modbus_read_file_record: rc %d, err %s, ", rc, modbus_strerror(errno)); + ASSERT_TRUE(((rc == -1) && (errno == EMBXILADD)), "FAILED (incorrect error handling)\n"); + /* Read three records to file 1 on server, starting at record 14 + * (before file end, but read goes over) */ + errno = 0; + rc = modbus_read_file_record(ctx, UT_FILE, UT_RECORDS_NB - 1, + UT_REGISTERS_NB, tab_file_records); + printf("2/4 modbus_read_file_record: rc %d, err %s, ", rc, modbus_strerror(errno)); + ASSERT_TRUE(((rc == -1) && (errno == EMBXILADD)), "FAILED (incorrect error handling)\n"); + /* Read three records to file 3 on server (which does not exist), + * starting at record 1 (which is OK) */ + errno = 0; + rc = modbus_read_file_record(ctx, UT_FILE + 2, 1, + UT_REGISTERS_NB, tab_file_records); + printf("3/4 modbus_read_file_record: rc %d, err %s, ", rc, modbus_strerror(errno)); + ASSERT_TRUE(((rc == -1) && (errno == EMBXILADD)), "FAILED (incorrect error handling)\n"); + /* Write three records to file 0 on server (which is not allowed), + * starting at record 1 (which is OK) */ + errno = 0; + rc = modbus_write_file_record(ctx, 0, 1, + UT_REGISTERS_NB, tab_file_records); + printf("4/4 modbus_read_file_record: rc %d, err %s, ", rc, modbus_strerror(errno)); + ASSERT_TRUE(((rc == -1) && (errno == EMBXILADD)), "FAILED (incorrect error handling)\n"); /* MASKS */ printf("1/1 Write mask: "); From 5d6d835ab0b8dde7c81cbf83a82b637d9d15a787 Mon Sep 17 00:00:00 2001 From: Richard Ash Date: Tue, 23 Jun 2020 18:03:43 +0100 Subject: [PATCH 17/21] Fix error handling for file records Correct the range check so that off-by-one reads do not access off the end of array. Change the returned exception to match the diagram in the ModBus specification, which says that invalid file number and invalid records both give Exception Code 02. --- src/modbus.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/modbus.c b/src/modbus.c index 40d73d3b9..09de957dd 100644 --- a/src/modbus.c +++ b/src/modbus.c @@ -1026,7 +1026,7 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, uint16_t record_addr = (req[offset + 5] << 8) + req[offset + 6]; uint16_t record_len = (req[offset + 7] << 8) + req[offset + 8]; - if (address < 1 || mapping_address < 0 || mapping_address > mb_mapping->nb_files) { + if (address < 1 || mapping_address < 0 || mapping_address >= mb_mapping->nb_files) { rsp_length = response_exception( ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp, FALSE, "Illegal file number %d in read_file_record\n", address); @@ -1034,7 +1034,7 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, /* index of last record (zero-based) is beyond end of file */ (record_addr + record_len) > mb_mapping->files[mapping_address].nb_records) { rsp_length = response_exception( - ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp, TRUE, + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp, TRUE, "Illegal request for record %d, len %d (in file len %d) in read_file_record\n", record_addr, record_len, mb_mapping->files[mapping_address].nb_records); } else { @@ -1057,7 +1057,7 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, uint16_t record_addr = (req[offset + 5] << 8) + req[offset + 6]; uint16_t record_len = (req[offset + 7] << 8) + req[offset + 8]; - if (address < 1 || mapping_address < 0 || mapping_address > mb_mapping->nb_files) { + if (address < 1 || mapping_address < 0 || mapping_address >= mb_mapping->nb_files) { rsp_length = response_exception( ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp, FALSE, "Illegal file number %d in write_file_record\n", address); @@ -1066,7 +1066,7 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req, /* index of last record (zero-based) is beyond end of file */ (record_addr + record_len) > mb_mapping->files[mapping_address].nb_records) { rsp_length = response_exception( - ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_VALUE, rsp, TRUE, + ctx, &sft, MODBUS_EXCEPTION_ILLEGAL_DATA_ADDRESS, rsp, TRUE, "Illegal record %d, len 0x%0X (max %d) in write_file_record\n", record_addr, record_len, mb_mapping->files[mapping_address].nb_records); } else { From d7656c5094d397722500834497850d8ec90c0eee Mon Sep 17 00:00:00 2001 From: Richard Ash Date: Fri, 18 Jul 2025 11:36:45 +0100 Subject: [PATCH 18/21] Correct error in documentation. --- docs/modbus_mapping_new.md | 2 +- docs/modbus_mapping_new_start_address.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/modbus_mapping_new.md b/docs/modbus_mapping_new.md index 11f3196bd..cbc42c476 100644 --- a/docs/modbus_mapping_new.md +++ b/docs/modbus_mapping_new.md @@ -14,7 +14,7 @@ modbus_mapping_t* modbus_mapping_new(int nb_bits, int nb_input_bits, ## Description -The *modbus_mapping_new()* function shall allocate six arrays to store bits, +The *modbus_mapping_new()* function shall allocate five arrays to store bits, input bits, registers, inputs registers, files and file records. The pointers are stored in *modbus_mapping_t* structure. All values of the arrays are initialized to zero. diff --git a/docs/modbus_mapping_new_start_address.md b/docs/modbus_mapping_new_start_address.md index 4f21ec2a3..90c6b2f49 100644 --- a/docs/modbus_mapping_new_start_address.md +++ b/docs/modbus_mapping_new_start_address.md @@ -18,7 +18,7 @@ modbus_mapping_t* modbus_mapping_new_start_address( ## Description -The `modbus_mapping_new_start_address()` function shall allocate six arrays to +The `modbus_mapping_new_start_address()` function shall allocate five arrays to store bits, input bits, registers, inputs registers, files and file records. The pointers are stored in *modbus_mapping_t* structure. All values of the arrays are initialized to zero. From 22bbe09b00d32e03f180cb94381fac396469d7ca Mon Sep 17 00:00:00 2001 From: Richard Ash Date: Fri, 18 Jul 2025 11:49:08 +0100 Subject: [PATCH 19/21] Reformat docs as markdown and correct spelling errors --- docs/modbus_read_file_record.md | 43 ++++++++++++++++++++++++++ docs/modbus_read_file_record.txt | 51 ------------------------------- docs/modbus_write_file_record.md | 39 +++++++++++++++++++++++ docs/modbus_write_file_record.txt | 49 ----------------------------- 4 files changed, 82 insertions(+), 100 deletions(-) create mode 100644 docs/modbus_read_file_record.md delete mode 100644 docs/modbus_read_file_record.txt create mode 100644 docs/modbus_write_file_record.md delete mode 100644 docs/modbus_write_file_record.txt diff --git a/docs/modbus_read_file_record.md b/docs/modbus_read_file_record.md new file mode 100644 index 000000000..9c533716a --- /dev/null +++ b/docs/modbus_read_file_record.md @@ -0,0 +1,43 @@ +# modbus_read_file_record + +## Name + +modbus_read_file_record - Read records from a file + +## Synopsis + +```c +int modbus_read_file_record(modbus_t *'ctx', int 'addr', int 'sub_addr', int 'nb', uint16_t * 'dest'); +``` + +## Description + +The *modbus_read_file_record()* function reads `nb` records from file +number `addr`, starting from record position `sub_addr` in the file. +The result of reading is stored in `dest` array as word values (16 bits). You +must take care to allocate enough memory to store the results in `dest` +- (at least `nb * sizeof(uint16_t)`). + +A file is an array of records, each of 16 bits. + +* A ModBus device may have up to 65535 files, addressed 1 to 65535 decimal. +* Each file contains 10000 records, addressed 0000 to 9999 decimal. + +A maximum of 124 records (`nb`) may be retrieved in a single request. + +This function uses the ModBus function code 0x14 (Read File Record). Note that +although the ModBus Specification allows for multiple non-contiguous reads in +the same file to be made in a single request, this function only supports a +single contiguous read request. + + +## Return value + +The function shall return the number of records read (i.e. the value of `nb`) if successful. +Otherwise it shall return -1 and set errno. + + +## See also + +- [modbus_read_register](modbus_read_register.md) +- [modbus_write_file_record](modbus_write_file_record.md) diff --git a/docs/modbus_read_file_record.txt b/docs/modbus_read_file_record.txt deleted file mode 100644 index c14d9751b..000000000 --- a/docs/modbus_read_file_record.txt +++ /dev/null @@ -1,51 +0,0 @@ -modbus_read_file_record(3) -========================== - - -NAME ----- -modbus_read_file_record - Read records from a file - - -SYNOPSIS --------- -*int modbus_read_file_record(modbus_t *'ctx', int 'addr', int 'sub_addr', int 'nb', uint16_t * 'dest');* - - -DESCRIPTION ------------ -The *modbus_read_file_record()* function reads _nb_ records from file -number _addr_, starting from record position _sub_addr_ in the file. -The result of reading is stored in _dest_ array as word values (16 bits). You -must take care to allocate enough memory to store the results in _dest_ -- (at least _nb_ * sizeof(uint16_t)). - -A file is an array of records, each of 16 bits. - -* A modbus device may have up to 65535 files, addressed 1 to 65535 decimal. -* Each file contains 10000 records, addressed 0000 to 9999 decimal. - -A maximum of 124 records (_nb_) may be retrived in a single request. - -This function uses the Modbus function code 0x14 (Read File Record). Note that -althrough the ModBus Specification allows for multiple non-contiguous reads in -the same file to be made in a single request, this function only supports a -single contiguous read request. - - -RETURN VALUE ------------- -The function shall return the number of records read (i.e. the value of _nb_) if successful. -Otherwise it shall return -1 and set errno. - - -SEE ALSO --------- -linkmb:modbus_read_register[3] -linkmb:modbus_write_file_register[3] - - -AUTHORS -------- -File record support by Kamil Wcislo , documentation -extended by Richard Ash at EA Technology. diff --git a/docs/modbus_write_file_record.md b/docs/modbus_write_file_record.md new file mode 100644 index 000000000..1fab55247 --- /dev/null +++ b/docs/modbus_write_file_record.md @@ -0,0 +1,39 @@ +modbus_write_file_record + +## Name + +modbus_write_file_record - Write records to a file + +## Synopsis + +```c +int modbus_write_file_record(modbus_t *'ctx', int 'addr', int 'sub_addr', int 'nb', const uint16_t * 'src'); +``` + +## Description + +The *modbus_write_file_record()* function writes the content of `nb` +records from the `src` array to file number `addr`, starting from position +`sub_addr` in the file. + +A file is an array of records, each of 16 bits. + +* A ModBus device may have up to 65535 files, addressed 1 to 65535 decimal. +* Each file contains 10000 records, addressed 0000 to 9999 decimal. + +A maximum of 124 records (`nb`) may be written in a single request. + +This function uses the ModBus function code 0x15 (Write File Record). Note that +although the ModBus Specification allows for multiple non-contiguous writes in +the same file to be made in a single request, this function only supports a +single contiguous write request. + +## Return value + +The function shall return the number of records written (i.e. the value of `nb`) if successful. +Otherwise it shall return -1 and set errno. + +## See also + +- [modbus_write_register](modbus_write_register.md) +- [modbus_read_file_record](modbus_read_file_record.md) diff --git a/docs/modbus_write_file_record.txt b/docs/modbus_write_file_record.txt deleted file mode 100644 index 66b715a65..000000000 --- a/docs/modbus_write_file_record.txt +++ /dev/null @@ -1,49 +0,0 @@ -modbus_write_file_record(3) -=========================== - - -NAME ----- -modbus_write_file_record - Write records to a file - - -SYNOPSIS --------- -*int modbus_write_file_record(modbus_t *'ctx', int 'addr', int 'sub_addr', int 'nb', const uint16_t * 'src');* - - -DESCRIPTION ------------ -The *modbus_write_file_record()* function writes the content of the _nb_ -records from the _src_ array to file number _addr_, starting from position -_sub_addr_ in the file. - -A file is an array of records, each of 16 bits. - -* A modbus device may have up to 65535 files, addressed 1 to 65535 decimal. -* Each file contains 10000 records, addressed 0000 to 9999 decimal. - -A maximum of 124 records (_nb_) may be written in a single request. - -This function uses the Modbus function code 0x15 (Write File Record). Note that -althrough the ModBus Specification allows for multiple non-contiguous writes in -the same file to be made in a single request, this function only supports a -single contiguous write request. - - -RETURN VALUE ------------- -The function shall return the number of records written (i.e. the value of _nb_) if successful. -Otherwise it shall return -1 and set errno. - - -SEE ALSO --------- -linkmb:modbus_write_register[3] -linkmb:modbus_read_file_register[3] - - -AUTHORS -------- -The libmodbus file support was added by Kamil Wcislo and -the documentation extended by Richard Ash at EA Technology. From 4170817f6d22e31db7c12336f8b1c9c5c409f9ff Mon Sep 17 00:00:00 2001 From: Richard Ash Date: Fri, 18 Jul 2025 14:05:03 +0100 Subject: [PATCH 20/21] Fix unit tests: add missing break; and better messages. --- src/modbus.c | 1 + tests/unit-test-client.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/modbus.c b/src/modbus.c index 622de7085..aa4513bd2 100644 --- a/src/modbus.c +++ b/src/modbus.c @@ -685,6 +685,7 @@ static int check_confirmation(modbus_t *ctx, uint8_t *req, uint8_t *rsp, int rsp case MODBUS_FC_WRITE_FILE_RECORD: req_nb_value = (req[offset + 7] << 8) + req[offset + 8]; rsp_nb_value = (rsp[offset + 7] << 8) + rsp[offset + 8]; + break; case MODBUS_FC_WRITE_SINGLE_COIL: case MODBUS_FC_WRITE_SINGLE_REGISTER: /* address in request and response must be equal */ diff --git a/tests/unit-test-client.c b/tests/unit-test-client.c index 2acb44d6d..90e705a90 100644 --- a/tests/unit-test-client.c +++ b/tests/unit-test-client.c @@ -335,14 +335,14 @@ int main(int argc, char *argv[]) rc = modbus_write_file_record(ctx, UT_FILE, UT_RECORD_ADDRESS, UT_REGISTERS_NB, UT_REGISTERS_TAB); printf("3/6 modbus_write_file_record: "); - ASSERT_TRUE(rc == UT_REGISTERS_NB, "FAILED (nb points %d)\n", rc); + ASSERT_TRUE(rc == UT_REGISTERS_NB, "FAILED (nb records written %d)\n", rc); /* Write four more records to file 1 on server, starting at record 5, * i.e. after previous records */ nb_points = UT_RECORD_ADDRESS + UT_REGISTERS_NB; rc = modbus_write_file_record(ctx, UT_FILE, nb_points, UT_FILE_RECORD_NB, UT_FILE_TAB); printf("4/6 modbus_write_file_record: "); - ASSERT_TRUE(rc == UT_FILE_RECORD_NB, "FAILED (nb points %d)\n", rc); + ASSERT_TRUE(rc == UT_FILE_RECORD_NB, "FAILED (nb records written %d)\n", rc); /* Read back all 7 records and make sure the values are correct. * Allocate and initialize the memory to store the registers as we need more. From 7fbd38d3ecc0e58a1428741bee36a4042dc89f11 Mon Sep 17 00:00:00 2001 From: Richard Ash Date: Fri, 18 Jul 2025 14:22:03 +0100 Subject: [PATCH 21/21] Add file record documentation to the index file. --- docs/index.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/index.md b/docs/index.md index 4e6923642..fc979ecb4 100644 --- a/docs/index.md +++ b/docs/index.md @@ -142,6 +142,7 @@ To read data: - [modbus_read_registers](modbus_read_registers.md) - [modbus_read_input_registers](modbus_read_input_registers.md) - [modbus_report_slave_id](modbus_report_slave_id.md) +- [modbus_read_file_record](modbus_read_file_record.md) To write data: @@ -149,6 +150,7 @@ To write data: - [modbus_write_register](modbus_write_register.md) - [modbus_write_bits](modbus_write_bits.md) - [modbus_write_registers](modbus_write_registers.md) +- [modbus_write_file_record](modbus_write_file_record.md) To write and read data in a single operation: