From ce1ad505e9ad577a3291d96309e3dea02c312e19 Mon Sep 17 00:00:00 2001 From: Valerio De Benedetto Date: Tue, 18 Jan 2022 20:03:16 +0100 Subject: [PATCH] Fixes, base server tests --- modbusino.c | 116 ++++++++++++++++++++++---------------- modbusino.h | 18 +++--- tests/modbusino_tests.c | 121 +++++++++++++++++++++++++++++++++------- tests/modbusino_tests.h | 63 +++++++++++++++++++++ 4 files changed, 241 insertions(+), 77 deletions(-) create mode 100644 tests/modbusino_tests.h diff --git a/modbusino.c b/modbusino.c index 9fba090..1499644 100644 --- a/modbusino.c +++ b/modbusino.c @@ -39,16 +39,19 @@ typedef struct req_state { } req_state; -uint64_t clock_ms() { +static uint64_t clock_ms() { return (uint64_t) clock() / CLOCKS_PER_MS; } int mbsn_create(mbsn_t* mbsn, mbsn_transport_conf transport_conf) { + if (!mbsn) + return MBSN_ERROR_INVALID_ARGUMENT; + memset(mbsn, 0, sizeof(mbsn_t)); mbsn->byte_timeout_ms = -1; - mbsn->response_timeout_ms = -1; + mbsn->read_timeout_ms = -1; mbsn->transport = transport_conf.transport; if (mbsn->transport != MBSN_TRANSPORT_RTU && mbsn->transport != MBSN_TRANSPORT_TCP) @@ -75,28 +78,32 @@ mbsn_error mbsn_server_create(mbsn_t* mbsn, uint8_t address, mbsn_transport_conf return MBSN_ERROR_INVALID_ARGUMENT; mbsn_error ret = mbsn_create(mbsn, transport_conf); + if (ret != MBSN_ERROR_NONE) + return ret; + mbsn->address_rtu = address; mbsn->callbacks = callbacks; - return ret; + + return MBSN_ERROR_NONE; } -void mbsn_set_byte_timeout(mbsn_t* mbsn, int64_t timeout_ms) { +void mbsn_set_read_timeout(mbsn_t* mbsn, int32_t timeout_ms) { + mbsn->read_timeout_ms = timeout_ms; +} + + +void mbsn_set_byte_timeout(mbsn_t* mbsn, int32_t timeout_ms) { mbsn->byte_timeout_ms = timeout_ms; } -void mbsn_set_response_timeout(mbsn_t* mbsn, int64_t timeout_ms) { - mbsn->response_timeout_ms = timeout_ms; -} - - void mbsn_client_set_server_address_rtu(mbsn_t* mbsn, uint8_t address) { mbsn->server_dest_address_rtu = address; } -uint16_t crc_add_n(uint16_t crc, const uint8_t* data, unsigned int length) { +static uint16_t crc_add_n(uint16_t crc, const uint8_t* data, unsigned int length) { for (int i = 0; i < length; i++) { crc ^= (uint16_t) data[i]; for (int j = 8; j != 0; j--) { @@ -113,12 +120,12 @@ uint16_t crc_add_n(uint16_t crc, const uint8_t* data, unsigned int length) { } -uint16_t crc_add_1(uint16_t crc, const uint8_t b) { +static uint16_t crc_add_1(uint16_t crc, const uint8_t b) { return crc_add_n(crc, &b, 1); } -uint16_t crc_add_2(uint16_t crc, const uint8_t w) { +static uint16_t crc_add_2(uint16_t crc, const uint8_t w) { return crc_add_n(crc, &w, 2); } @@ -146,19 +153,12 @@ mbsn_error read(uint8_t* buf, uint64_t len, uint64_t byte_timeout_ms, mbsn_error */ -mbsn_error recv_n(mbsn_t* mbsn, uint8_t* buf, uint32_t count, uint16_t* crc) { - //uint64_t start = clock_ms(); +static mbsn_error recv_n(mbsn_t* mbsn, uint8_t* buf, uint32_t count, uint16_t* crc) { int r = 0; while (r != count) { - int ret = mbsn->transport_read_byte(buf + r); - - /* - if (mbsn->byte_timeout_ms > 0 && clock_ms() - start >= mbsn->byte_timeout_ms) - return MBSN_ERROR_TIMEOUT; - */ - + int ret = mbsn->transport_read_byte(buf + r, mbsn->byte_timeout_ms); if (ret == 0) - continue; + return MBSN_ERROR_TIMEOUT; else if (ret != 1) return MBSN_ERROR_TRANSPORT; @@ -171,12 +171,12 @@ mbsn_error recv_n(mbsn_t* mbsn, uint8_t* buf, uint32_t count, uint16_t* crc) { } -mbsn_error recv_1(mbsn_t* mbsn, uint8_t* b, uint16_t* crc) { +static mbsn_error recv_1(mbsn_t* mbsn, uint8_t* b, uint16_t* crc) { return recv_n(mbsn, b, 1, crc); } -mbsn_error recv_2(mbsn_t* mbsn, uint16_t* w, uint16_t* crc) { +static mbsn_error recv_2(mbsn_t* mbsn, uint16_t* w, uint16_t* crc) { mbsn_error err = recv_n(mbsn, (uint8_t*) w, 2, crc); if (err != MBSN_ERROR_NONE) return err; @@ -231,12 +231,12 @@ mbsn_error send(mbsn_t* mbsn) { */ -mbsn_error send_n(mbsn_t* mbsn, uint8_t* buf, uint32_t count, uint16_t* crc) { +static mbsn_error send_n(mbsn_t* mbsn, uint8_t* buf, uint32_t count, uint16_t* crc) { int w = 0; while (w != count) { - int ret = mbsn->transport_write_byte(buf[w]); + int ret = mbsn->transport_write_byte(buf[w], mbsn->read_timeout_ms); if (ret == 0) - continue; + return MBSN_ERROR_TIMEOUT; else if (ret != 1) return MBSN_ERROR_TRANSPORT; @@ -250,18 +250,18 @@ mbsn_error send_n(mbsn_t* mbsn, uint8_t* buf, uint32_t count, uint16_t* crc) { } -mbsn_error send_1(mbsn_t* mbsn, uint8_t b, uint16_t* crc) { +static mbsn_error send_1(mbsn_t* mbsn, uint8_t b, uint16_t* crc) { return send_n(mbsn, &b, 1, crc); } -mbsn_error send_2(mbsn_t* mbsn, uint16_t w, uint16_t* crc) { +static mbsn_error send_2(mbsn_t* mbsn, uint16_t w, uint16_t* crc) { w = HTONS(w); return send_n(mbsn, (uint8_t*) &w, 2, crc); } -mbsn_error send_mbap(mbsn_t* mbsn, req_state* s, uint16_t data_length) { +static mbsn_error send_mbap(mbsn_t* mbsn, req_state* s, uint16_t data_length) { mbsn_error err = send_2(mbsn, s->transaction_id, NULL); if (err != MBSN_ERROR_NONE) return err; @@ -283,7 +283,7 @@ mbsn_error send_mbap(mbsn_t* mbsn, req_state* s, uint16_t data_length) { } -mbsn_error send_req_header(mbsn_t* mbsn, req_state* s, uint8_t data_length, uint16_t* crc) { +static mbsn_error send_req_header(mbsn_t* mbsn, req_state* s, uint8_t data_length, uint16_t* crc) { if (mbsn->transport == MBSN_TRANSPORT_RTU) { mbsn_error err = send_1(mbsn, s->client_id, crc); if (err != MBSN_ERROR_NONE) @@ -301,7 +301,7 @@ mbsn_error send_req_header(mbsn_t* mbsn, req_state* s, uint8_t data_length, uint } -mbsn_error send_req_footer(mbsn_t* mbsn, uint16_t crc) { +static mbsn_error send_req_footer(mbsn_t* mbsn, uint16_t crc) { if (mbsn->transport == MBSN_TRANSPORT_RTU) return send_2(mbsn, crc, NULL); @@ -309,7 +309,7 @@ mbsn_error send_req_footer(mbsn_t* mbsn, uint16_t crc) { } -mbsn_error handle_exception(mbsn_t* mbsn, req_state* s, uint8_t exception) { +static mbsn_error handle_exception(mbsn_t* mbsn, req_state* s, uint8_t exception) { uint16_t crc = 0xFFFF; mbsn_error err = send_req_header(mbsn, s, 1, &crc); if (err != MBSN_ERROR_NONE) @@ -327,7 +327,8 @@ mbsn_error handle_exception(mbsn_t* mbsn, req_state* s, uint8_t exception) { } -mbsn_error handle_read_discrete(mbsn_t* mbsn, req_state* s, mbsn_error (*callback)(uint16_t, uint16_t, mbsn_bitfield)) { +static mbsn_error handle_read_discrete(mbsn_t* mbsn, req_state* s, + mbsn_error (*callback)(uint16_t, uint16_t, mbsn_bitfield)) { uint16_t addr; mbsn_error err = recv_2(mbsn, &addr, &s->crc); if (err != MBSN_ERROR_NONE) @@ -393,7 +394,8 @@ mbsn_error handle_read_discrete(mbsn_t* mbsn, req_state* s, mbsn_error (*callbac } -mbsn_error handle_read_registers(mbsn_t* mbsn, req_state* s, mbsn_error (*callback)(uint16_t, uint16_t, uint16_t*)) { +static mbsn_error handle_read_registers(mbsn_t* mbsn, req_state* s, + mbsn_error (*callback)(uint16_t, uint16_t, uint16_t*)) { uint16_t addr; mbsn_error err = recv_2(mbsn, &addr, &s->crc); if (err != MBSN_ERROR_NONE) @@ -463,27 +465,27 @@ mbsn_error handle_read_registers(mbsn_t* mbsn, req_state* s, mbsn_error (*callba } -mbsn_error handle_read_coils(mbsn_t* mbsn, req_state* s) { +static mbsn_error handle_read_coils(mbsn_t* mbsn, req_state* s) { return handle_read_discrete(mbsn, s, mbsn->callbacks.read_coils); } -mbsn_error handle_read_discrete_inputs(mbsn_t* mbsn, req_state* s) { +static mbsn_error handle_read_discrete_inputs(mbsn_t* mbsn, req_state* s) { return handle_read_discrete(mbsn, s, mbsn->callbacks.read_discrete_inputs); } -mbsn_error handle_read_holding_registers(mbsn_t* mbsn, req_state* s) { +static mbsn_error handle_read_holding_registers(mbsn_t* mbsn, req_state* s) { return handle_read_registers(mbsn, s, mbsn->callbacks.read_holding_registers); } -mbsn_error handle_read_input_registers(mbsn_t* mbsn, req_state* s) { +static mbsn_error handle_read_input_registers(mbsn_t* mbsn, req_state* s) { return handle_read_registers(mbsn, s, mbsn->callbacks.read_input_registers); } -mbsn_error handle_write_single_coil(mbsn_t* mbsn, req_state* s) { +static mbsn_error handle_write_single_coil(mbsn_t* mbsn, req_state* s) { uint16_t addr; mbsn_error err = recv_2(mbsn, &addr, &s->crc); if (err != MBSN_ERROR_NONE) @@ -540,7 +542,7 @@ mbsn_error handle_write_single_coil(mbsn_t* mbsn, req_state* s) { } -mbsn_error handle_write_single_register(mbsn_t* mbsn, req_state* s) { +static mbsn_error handle_write_single_register(mbsn_t* mbsn, req_state* s) { uint16_t addr; mbsn_error err = recv_2(mbsn, &addr, &s->crc); if (err != MBSN_ERROR_NONE) @@ -594,7 +596,7 @@ mbsn_error handle_write_single_register(mbsn_t* mbsn, req_state* s) { } -mbsn_error handle_write_multiple_coils(mbsn_t* mbsn, req_state* s) { +static mbsn_error handle_write_multiple_coils(mbsn_t* mbsn, req_state* s) { uint16_t addr; mbsn_error err = recv_2(mbsn, &addr, &s->crc); if (err != MBSN_ERROR_NONE) @@ -671,7 +673,7 @@ mbsn_error handle_write_multiple_coils(mbsn_t* mbsn, req_state* s) { } -mbsn_error handle_write_multiple_registers(mbsn_t* mbsn, req_state* s) { +static mbsn_error handle_write_multiple_registers(mbsn_t* mbsn, req_state* s) { uint16_t addr; mbsn_error err = recv_2(mbsn, &addr, &s->crc); if (err != MBSN_ERROR_NONE) @@ -755,11 +757,22 @@ int mbsn_server_receive(mbsn_t* mbsn) { req_state s = {0}; s.crc = 0xFFFF; + // We should wait for the read timeout for the first message byte + int32_t old_byte_timeout = mbsn->byte_timeout_ms; + mbsn->byte_timeout_ms = mbsn->read_timeout_ms; + if (mbsn->transport == MBSN_TRANSPORT_RTU) { uint8_t id; mbsn_error err = recv_1(mbsn, &id, &s.crc); - if (err != 0) - return err; + + mbsn->byte_timeout_ms = old_byte_timeout; + + if (err != 0) { + if (err == MBSN_ERROR_TIMEOUT) + return MBSN_ERROR_NONE; + else + return err; + } // Check if request is for us if (id == 0) @@ -775,8 +788,15 @@ int mbsn_server_receive(mbsn_t* mbsn) { } else if (mbsn->transport == MBSN_TRANSPORT_TCP) { mbsn_error err = recv_2(mbsn, &s.transaction_id, NULL); - if (err != MBSN_ERROR_NONE) - return err; + + mbsn->byte_timeout_ms = old_byte_timeout; + + if (err != 0) { + if (err == MBSN_ERROR_TIMEOUT) + return MBSN_ERROR_NONE; + else + return err; + } uint16_t protocol_id = 0xFFFF; err = recv_2(mbsn, &protocol_id, NULL); @@ -854,4 +874,4 @@ int mbsn_server_receive(mbsn_t* mbsn) { } return MBSN_ERROR_NONE; -} \ No newline at end of file +} diff --git a/modbusino.h b/modbusino.h index a1961eb..cbc3620 100644 --- a/modbusino.h +++ b/modbusino.h @@ -37,8 +37,8 @@ typedef enum mbsn_transport { typedef struct mbsn_transport_conf { mbsn_transport transport; - int (*read_byte)(uint8_t* b); - int (*write_byte)(uint8_t b); + int (*read_byte)(uint8_t* b, uint32_t); + int (*write_byte)(uint8_t b, uint32_t); } mbsn_transport_conf; @@ -70,12 +70,12 @@ typedef struct mbsn_t { // Private fields mbsn_callbacks callbacks; - int64_t byte_timeout_ms; - int64_t response_timeout_ms; + int32_t byte_timeout_ms; + int32_t read_timeout_ms; mbsn_transport transport; - mbsn_error (*transport_read_byte)(uint8_t*); - mbsn_error (*transport_write_byte)(uint8_t); + mbsn_error (*transport_read_byte)(uint8_t*, uint32_t); + mbsn_error (*transport_write_byte)(uint8_t, uint32_t); uint8_t address_rtu; uint8_t server_dest_address_rtu; @@ -87,13 +87,13 @@ mbsn_error mbsn_client_create(mbsn_t* mbsn, mbsn_transport_conf transport_conf); mbsn_error mbsn_server_create(mbsn_t* mbsn, uint8_t address, mbsn_transport_conf transport_conf, mbsn_callbacks callbacks); -void mbsn_set_response_timeout(mbsn_t* mbsn, int64_t timeout_ms); +void mbsn_set_read_timeout(mbsn_t* mbsn, int32_t timeout_ms); -void mbsn_set_byte_timeout(mbsn_t* mbsn, int64_t timeout_ms); +void mbsn_set_byte_timeout(mbsn_t* mbsn, int32_t timeout_ms); void mbsn_client_set_server_address(mbsn_t* mbsn, uint16_t address); -mbsn_error mbsn_server_poll(mbsn_t* mbsn); +mbsn_error mbsn_server_receive(mbsn_t* mbsn); mbsn_error mbsn_read_coils(uint16_t address, uint16_t quantity, uint8_t* coils_out); diff --git a/tests/modbusino_tests.c b/tests/modbusino_tests.c index 2cb4fc9..c364ddb 100644 --- a/tests/modbusino_tests.c +++ b/tests/modbusino_tests.c @@ -1,34 +1,115 @@ +#include "modbusino_tests.h" #include "modbusino.h" #include #include #include -mbsn_error read_byte(uint8_t* b) {} -mbsn_error write_byte(uint8_t b) {} +void test_server_create(mbsn_transport transport) { + mbsn_t mbsn; + mbsn_error err; + + mbsn_transport_conf transport_conf_empty = {.transport = transport, + .read_byte = read_byte_empty, + .write_byte = write_byte_empty}; + + should("create a modbus server"); + reset(mbsn); + err = mbsn_server_create(&mbsn, TEST_SERVER_ADDR, transport_conf_empty, (mbsn_callbacks){}); + check(err); + + should("check parameters and fail to create a modbus server"); + reset(mbsn); + err = mbsn_server_create(NULL, TEST_SERVER_ADDR, transport_conf_empty, (mbsn_callbacks){}); + expect(err == MBSN_ERROR_INVALID_ARGUMENT); + + reset(mbsn); + err = mbsn_server_create(&mbsn, 0, transport_conf_empty, (mbsn_callbacks){}); + expect(err == MBSN_ERROR_INVALID_ARGUMENT); + + reset(mbsn); + mbsn_transport_conf t = transport_conf_empty; + t.transport = 3; + err = mbsn_server_create(&mbsn, 0, t, (mbsn_callbacks){}); + expect(err == MBSN_ERROR_INVALID_ARGUMENT); + + reset(mbsn); + t = transport_conf_empty; + t.read_byte = NULL; + err = mbsn_server_create(&mbsn, 0, t, (mbsn_callbacks){}); + expect(err == MBSN_ERROR_INVALID_ARGUMENT); + + reset(mbsn); + t = transport_conf_empty; + t.write_byte = NULL; + err = mbsn_server_create(&mbsn, 0, t, (mbsn_callbacks){}); + expect(err == MBSN_ERROR_INVALID_ARGUMENT); +}; + + +void test_server_receive_base(mbsn_transport transport) { + mbsn_t mbsn; + mbsn_error err; + mbsn_transport_conf transport_conf; + + should("honor read_timeout and return normally"); + reset(mbsn); + transport_conf.transport = transport; + transport_conf.read_byte = read_byte_timeout_1s; + transport_conf.write_byte = write_byte_empty; + + const int32_t read_timeout_ms = 250; + + err = mbsn_server_create(&mbsn, TEST_SERVER_ADDR, transport_conf, (mbsn_callbacks){}); + check(err); + + mbsn_set_read_timeout(&mbsn, read_timeout_ms); + mbsn_set_byte_timeout(&mbsn, -1); + + const int polls = 5; + for (int i = 0; i < polls; i++) { + uint64_t start = now_ms(); + err = mbsn_server_receive(&mbsn); + check(err); + + uint64_t diff = now_ms() - start; + + if (diff < read_timeout_ms) { + fail(); + } + } + + should("honor byte_timeout and return MBSN_ERROR_TIMEOUT"); + reset(mbsn); + transport_conf.transport = transport; + transport_conf.read_byte = read_byte_timeout_third; + transport_conf.write_byte = write_byte_empty; + + const int32_t byte_timeout_ms = 250; + + err = mbsn_server_create(&mbsn, TEST_SERVER_ADDR, transport_conf, (mbsn_callbacks){}); + check(err); + + mbsn_set_read_timeout(&mbsn, -1); + mbsn_set_byte_timeout(&mbsn, byte_timeout_ms); + + err = mbsn_server_receive(&mbsn); + expect(err == MBSN_ERROR_TIMEOUT); +} + int main() { - mbsn_t mbsn = {0}; - mbsn_error err = mbsn_server_create( - &mbsn, - (mbsn_transport_conf){.transport = MBSN_TRANSPORT_TCP, .read_byte = read_byte, .write_byte = write_byte}); + should("create a RTU modbus server"); + test(test_server_create(MBSN_TRANSPORT_TCP)); - assert(err == MBSN_ERROR_NONE); + should("create a TCP modbus server"); + test(test_server_create(MBSN_TRANSPORT_TCP)); - mbsn_bitfield bf; - memset(bf, 0, sizeof(bf)); + should("receive no messages as RTU server without failing"); + test(test_server_receive_base(MBSN_TRANSPORT_RTU)); - mbsn_bitfield_write(bf, 7, true); - printf("%d\n", mbsn_bitfield_read(bf, 7)); - - mbsn_bitfield_write(bf, 22, true); - printf("%d\n", mbsn_bitfield_read(bf, 22)); - - mbsn_bitfield_write(bf, 0, true); - printf("%d\n", mbsn_bitfield_read(bf, 0)); - - mbsn_bitfield_write(bf, 24, true); - printf("%d\n", mbsn_bitfield_read(bf, 24)); + should("receive no messages as TCP server without failing"); + test(test_server_receive_base(MBSN_TRANSPORT_TCP)); return 0; } diff --git a/tests/modbusino_tests.h b/tests/modbusino_tests.h new file mode 100644 index 0000000..e941f4a --- /dev/null +++ b/tests/modbusino_tests.h @@ -0,0 +1,63 @@ +#include "modbusino.h" +#include +#include + +unsigned int nesting = 0; + +#define fail() (assert(false)) + +#define should(s) \ + { \ + for (int i = 0; i < nesting; i++) { \ + printf("\t"); \ + } \ + printf("Should %s\n", (s)); \ + } + +#define expect(c) assert(c) + +#define check(err) (expect((err) == MBSN_ERROR_NONE)) + +#define reset(mbsn) (memset(&(mbsn), 0, sizeof(mbsn_t))) + +#define test(f) (nesting++, (f), nesting--) + + +const uint8_t TEST_SERVER_ADDR = 1; + +static uint64_t now_ms() { + struct timespec ts = {0, 0}; + clock_gettime(CLOCK_MONOTONIC_RAW, &ts); + return (uint64_t) (ts.tv_sec) * 1000 + (uint64_t) (ts.tv_nsec) / 1000000; +} + +int read_byte_empty(uint8_t* b, uint32_t timeout) { + return 0; +} + +int write_byte_empty(uint8_t b, uint32_t timeout) { + return 0; +} + +int read_byte_timeout_1s(uint8_t* b, uint32_t timeout) { + usleep(timeout * 1000); + return 0; +} + +int read_byte_timeout_third(uint8_t* b, uint32_t timeout) { + static int stage = 0; + switch (stage) { + case 0: + case 1: + *b = 1; + stage++; + return 1; + case 2: + usleep(timeout * 1000 + 100); + stage = 0; + return 0; + default: + stage = 0; + return -1; + } +}