From 34a7c0ca08683eb83d6b6b3d5a6a8fb2f7d5b918 Mon Sep 17 00:00:00 2001 From: Christopher Peplin Date: Fri, 27 Dec 2013 18:28:03 -0500 Subject: [PATCH] Add skeleton tests for receiving and sending ISO-TP messages. --- Makefile | 10 +++--- README.mkd | 2 ++ src/isotp/isotp.c | 44 ++++++++++++++++++++++++++- src/isotp/isotp.h | 81 ++++++++++++++++++++++++++++++++++++++----------- tests/common.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++ tests/test_receive.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++ tests/test_send.c | 61 +++++++++++++++++++++++++++++++++++++ tests/tests.c | 30 ------------------ 8 files changed, 341 insertions(+), 52 deletions(-) create mode 100644 tests/common.c create mode 100644 tests/test_receive.c create mode 100644 tests/test_send.c delete mode 100644 tests/tests.c diff --git a/Makefile b/Makefile index 0dd93599..847196ab 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ CC = gcc INCLUDES = -Isrc -Ideps/canutil/src -CFLAGS = $(INCLUDES) -c -w -Wall -Werror -g -ggdb +CFLAGS = $(INCLUDES) -c -w -Wall -Werror -g -ggdb -std=c99 LDFLAGS = LDLIBS = -lcheck @@ -16,11 +16,13 @@ ifneq ($(OSTYPE),Darwin) endif SRC = $(wildcard src/**/*.c) -SRC = $(wildcard deps/**/*.c) +SRC += $(wildcard deps/**/*.c) OBJS = $(SRC:.c=.o) -TEST_SRC = $(wildcard $(TEST_DIR)/*.c) +TEST_SRC = $(wildcard $(TEST_DIR)/test_*.c) TEST_OBJS = $(TEST_SRC:.c=.o) TESTS=$(patsubst %.c,%.bin,$(TEST_SRC)) +TEST_SUPPORT_SRC = $(TEST_DIR)/common.c +TEST_SUPPORT_OBJS = $(TEST_SUPPORT_SRC:.c=.o) all: $(OBJS) @@ -29,7 +31,7 @@ test: $(TESTS) @export SHELLOPTS @sh runtests.sh $(TEST_DIR) -$(TEST_DIR)/%.bin: $(TEST_DIR)/%.o $(OBJS) +$(TEST_DIR)/%.bin: $(TEST_DIR)/%.o $(OBJS) $(TEST_SUPPORT_OBJS) @mkdir -p $(dir $@) $(CC) $(LDFLAGS) $(CC_SYMBOLS) $(INCLUDES) -o $@ $^ $(LDLIBS) diff --git a/README.mkd b/README.mkd index 530ebad5..2371561d 100644 --- a/README.mkd +++ b/README.mkd @@ -12,6 +12,8 @@ ISO-TP (ISO 15765-2) Support Library in C // TODO should handlers take a context? depends on how we want to use this +// TODO add an optional dispatcher to handle multiple open requests + ## Testing The library includes a test suite that uses the `check` C unit test library. diff --git a/src/isotp/isotp.c b/src/isotp/isotp.c index 1b7f398b..da7fa29d 100644 --- a/src/isotp/isotp.c +++ b/src/isotp/isotp.c @@ -1,3 +1,9 @@ +#include + +const uint16_t MAX_ISO_TP_MESSAGE_SIZE = 4096; +const uint16_t MAX_CAN_FRAME_SIZE = 8; +const uint8_t ISO_TP_DEFAULT_RESPONSE_TIMEOUT = 100; +const bool ISO_TP_DEFAULT_FRAME_PADDING_STATUS = true; void isotp_receive_can_frame(const uint16_t arbitration_id, const uint8_t* data, const uint8_t length) { @@ -10,5 +16,41 @@ bool isotp_send(const uint8_t* payload, uint16_t payload_size) { // we determine if it's single/multi frame and start the send } -void isotp_set_timeout(uint16_t timeout) { +void isotp_set_timeout(IsoTpHandler* handler, uint16_t timeout_ms) { + handler->timeout_ms = timeout_ms; +} + +IsoTpShims isotp_init_shims(LogShim log, SendCanMessageShim send_can_message, + SetTimerShim set_timer) { + IsoTpShims shims = { + log: log, + send_can_message: send_can_message, + set_timer: set_timer + }; + return shims; +} + +IsoTpHandler isotp_init(IsoTpShims* shims, uint16_t arbitration_id, + IsoTpMessageReceivedHandler message_received_callback, + IsoTpMessageSentHandler message_sent_callback, + IsoTpCanFrameSentHandler can_frame_sent_callback) { + IsoTpHandler handler = { + shims: shims, + message_received_callback: message_received_callback, + message_sent_callback: message_sent_callback, + can_frame_sent_callback: can_frame_sent_callback, + timeout_ms: ISO_TP_DEFAULT_RESPONSE_TIMEOUT, + frame_padding: ISO_TP_DEFAULT_FRAME_PADDING_STATUS, + sending: false + }; + return handler; +} + +// TODO this would be better as a "isotp_message_to_string" +void log_isotp_message(const uint16_t arbitration_id, + const uint8_t* payload, const uint16_t size) { + debug("ID: 0x%02x, Payload:", arbitration_id); + for(int i = 0; i < size; i++) { + debug("0x%x", payload[i]); + } } diff --git a/src/isotp/isotp.h b/src/isotp/isotp.h index 923d5926..c500f6e3 100644 --- a/src/isotp/isotp.h +++ b/src/isotp/isotp.h @@ -1,50 +1,93 @@ #ifndef __ISOTP_H__ #define __ISOTP_H__ +#include +#include + #ifdef __cplusplus extern "C" { #endif -struct { +const uint16_t MAX_ISO_TP_MESSAGE_SIZE; +const uint16_t MAX_CAN_FRAME_SIZE; +const uint8_t ISO_TP_DEFAULT_RESPONSE_TIMEOUT; +const bool ISO_TP_DEFAULT_FRAME_PADDING_STATUS; + +typedef void (*LogShim)(const char* message); +typedef bool (*SendCanMessageShim)(const uint16_t arbitration_id, const uint8_t* data, + const uint8_t size); +typedef bool (*SetTimerShim)(uint16_t time_ms, void (*callback)); + +typedef void (*IsoTpMessageReceivedHandler)(const uint16_t arbitration_id, + const uint8_t* payload, const uint16_t size); +typedef void (*IsoTpMessageSentHandler)(const bool success, + const uint16_t arbitration_id, const uint8_t* payload, + const uint16_t size); +typedef void (*IsoTpCanFrameSentHandler)(const uint16_t arbitration_id, + const uint8_t* payload, const uint8_t size); + +// TODO this is really necessary and it would leak this library into the user's +// code +// typedef struct { + // const uint16_t arbitration_id; + // const uint8_t* payload; + // const uint16_t size; +// } IsoTpMessage; + +typedef struct { + LogShim log; + SendCanMessageShim send_can_message; + SetTimerShim set_timer; +} IsoTpShims; + +typedef struct { + IsoTpShims* shims; uint16_t arb_id; - void* message_received_callback; - void* message_sent_callback; - void* can_frame_sent_callback; + IsoTpMessageReceivedHandler message_received_callback; + IsoTpMessageSentHandler message_sent_callback; + IsoTpCanFrameSentHandler can_frame_sent_callback; // Private uint16_t timeout_ms; - bool framePadding; + bool frame_padding; uint8_t* receive_buffer; uint16_t received_buffer_size; uint16_t incoming_message_size; bool sending; - // TODO timer + // TODO timer callback } IsoTpHandler; -enum { +typedef enum { PCI_SINGLE, PCI_FIRST_FRAME, PCI_CONSECUTIVE_FRAME, PCI_FLOW_CONTROL_FRAME } IsoTpProtocolControlInformation; -enum { +typedef enum { PCI_FLOW_STATUS_CONTINUE, PCI_FLOW_STATUS_WAIT, PCI_FLOW_STATUS_OVERFLOW } IsoTpFlowStatus; -const uint16_t MAX_ISO_TP_MESSAGE_SIZE = 4096; -const uint16_t MAX_CAN_FRAME_SIZE = 8; -const uint8_t ISO_TP_DEFAULT_RESPONSE_TIMEOUT = 100; -const bool ISO_TP_DEFAULT_FRAME_PADDING_STATUS = true; +IsoTpShims isotp_init_shims(LogShim log, + SendCanMessageShim send_can_message, + SetTimerShim set_timer); -IsoTpHandler isotp_init(uint16_t arbitration_id, - IsoTpMessageReceivedHandler* message_received_callback, - IsoTpMessageSentHandler* message_sent_callback, - IsoTpCanFrameSentHandler* can_frame_sent_callback); +IsoTpHandler isotp_init(IsoTpShims* shims, + uint16_t arbitration_id, + IsoTpMessageReceivedHandler message_received_callback, + IsoTpMessageSentHandler message_sent_callback, + IsoTpCanFrameSentHandler can_frame_sent_callback); -void isotp_set_timeout(uint16_t timeout); +/* Public: Change the timeout for waiting on an ISO-TP response frame. + * + * If this function is not used, the conventional 100ms is used by default. + * + * handler - the ISO-TP handler to modify. + * timeout - the new timeout in milliseconds. + */ +void isotp_set_timeout(IsoTpHandler* handler, uint16_t timeout_ms); // TODO we have to make sure to copy the payload internall if it's more than 1 // frame, the soure could go out of scope @@ -55,6 +98,10 @@ void isotp_receive_can_frame(const uint16_t arbitration_id, const uint8_t* data, void isotp_destroy(IsoTpHandler* handler); +void log_isotp_message(const uint16_t arbitration_id, const uint8_t* payload, + const uint16_t size); + + #ifdef __cplusplus } #endif diff --git a/tests/common.c b/tests/common.c new file mode 100644 index 00000000..fb26c4af --- /dev/null +++ b/tests/common.c @@ -0,0 +1,86 @@ +#include +#include +#include +#include +#include + +IsoTpShims SHIMS; +IsoTpHandler ISOTP_HANDLER; + +uint16_t last_can_frame_sent_arb_id; +uint8_t last_can_payload_sent; +uint8_t last_can_payload_size; +bool can_frame_was_sent; + +bool message_was_received; +uint16_t last_message_received_arb_id; +uint8_t* last_message_received_payload; +uint8_t last_message_received_payload_size; + +uint16_t last_message_sent_arb_id; +bool last_message_sent_status; +uint8_t* last_message_sent_payload; +uint8_t last_message_sent_payload_size; + +void debug(const char* format, ...) { + va_list args; + va_start(args, format); + vprintf(format, args); + va_end(args); +} + +void mock_send_can(const uint16_t arbitration_id, const uint8_t* data, + const uint8_t size) { +} + +void mock_set_timer(uint16_t time_ms, void (*callback)) { +} + +void message_received(const uint16_t arbitration_id, const uint8_t* payload, + const uint16_t size) { + debug("Received ISO-TP message:"); + log_isotp_message(arbitration_id, payload, size); + last_message_received_arb_id = arbitration_id; + last_message_received_payload_size = size; + memcpy(last_message_received_payload, payload, size); +} + +void message_sent(const bool success, const uint16_t arbitration_id, + const uint8_t* payload, const uint16_t size) { + if(success) { + debug("Sent ISO-TP message:"); + } else { + debug("Unable to send ISO-TP message:"); + } + log_isotp_message(arbitration_id, payload, size); + + message_was_received = true; + last_message_sent_arb_id = arbitration_id; + last_message_sent_payload_size = size; + memcpy(last_message_sent_payload, payload, size); +} + +void can_frame_sent(const uint16_t arbitration_id, + const uint8_t* payload, const uint8_t size) { + debug("Sent CAN Frame:"); + log_isotp_message(arbitration_id, payload, size); + for(int i = 0; i < size; i++) { + debug("0x%x", payload[i]); + } + + can_frame_was_sent = true; + last_can_frame_sent_arb_id = arbitration_id; + last_can_payload_sent = size; + memcpy(last_can_payload_sent, payload, size); +} + +void setup() { + SHIMS = isotp_init_shims(debug, mock_send_can, mock_set_timer); + ISOTP_HANDLER = isotp_init(&SHIMS, 0x2a, message_received, message_sent, + can_frame_sent); + last_message_received_payload = malloc(MAX_ISO_TP_MESSAGE_SIZE); + last_message_sent_status = false; + message_was_received = false; + can_frame_was_sent = false; +} + diff --git a/tests/test_receive.c b/tests/test_receive.c new file mode 100644 index 00000000..4134cfd6 --- /dev/null +++ b/tests/test_receive.c @@ -0,0 +1,79 @@ +#include +#include +#include +#include +#include +#include + +extern IsoTpShims SHIMS; +extern IsoTpHandler ISOTP_HANDLER; + +extern uint16_t last_can_frame_sent_arb_id; +extern uint8_t last_can_payload_sent; +extern uint8_t last_can_payload_size; +extern bool can_frame_was_sent; + +extern bool message_was_received; +extern uint16_t last_message_received_arb_id; +extern uint8_t* last_message_received_payload; +extern uint8_t last_message_received_payload_size; + +extern uint16_t last_message_sent_arb_id; +extern bool last_message_sent_status; +extern uint8_t* last_message_sent_payload; +extern uint8_t last_message_sent_payload_size; + +extern void setup(); + +START_TEST (test_receive_wrong_id) +{ + const uint8_t data[8] = {0}; + isotp_receive_can_frame(0x100, data, sizeof(data)); + fail_if(message_was_received); +} +END_TEST + +START_TEST (test_receive_bad_pci) +{ + // 4 is a reserved number for the PCI field - only 0-3 are allowed + const uint8_t data[8] = {0x40}; + isotp_receive_can_frame(0x2a, data, sizeof(data)); + fail_if(message_was_received); +} +END_TEST + +START_TEST (test_receive_single_frame) +{ + const uint8_t data[8] = {0x0, 0x12, 0x34}; + isotp_receive_can_frame(0x2a, data, sizeof(data)); + fail_unless(message_was_received); + ck_assert_int_eq(last_message_received_arb_id, 0x2a); + ck_assert_int_eq(last_message_received_payload_size, 2); + ck_assert_int_eq(last_message_received_payload[0], 0x12); + ck_assert_int_eq(last_message_received_payload[0], 0x34); +} +END_TEST + +Suite* testSuite(void) { + Suite* s = suite_create("iso15765"); + TCase *tc_core = tcase_create("receive"); + tcase_add_checked_fixture(tc_core, setup, NULL); + tcase_add_test(tc_core, test_receive_wrong_id); + tcase_add_test(tc_core, test_receive_bad_pci); + tcase_add_test(tc_core, test_receive_single_frame); + suite_add_tcase(s, tc_core); + + return s; +} + +int main(void) { + int numberFailed; + Suite* s = testSuite(); + SRunner *sr = srunner_create(s); + // Don't fork so we can actually use gdb + srunner_set_fork_status(sr, CK_NOFORK); + srunner_run_all(sr, CK_NORMAL); + numberFailed = srunner_ntests_failed(sr); + srunner_free(sr); + return (numberFailed == 0) ? 0 : 1; +} diff --git a/tests/test_send.c b/tests/test_send.c new file mode 100644 index 00000000..144e040f --- /dev/null +++ b/tests/test_send.c @@ -0,0 +1,61 @@ +#include +#include +#include +#include +#include +#include + +extern IsoTpShims SHIMS; +extern IsoTpHandler ISOTP_HANDLER; + +extern uint16_t last_can_frame_sent_arb_id; +extern uint8_t last_can_payload_sent; +extern uint8_t last_can_payload_size; +extern bool can_frame_was_sent; + +extern bool message_was_received; +extern uint16_t last_message_received_arb_id; +extern uint8_t* last_message_received_payload; +extern uint8_t last_message_received_payload_size; + +extern uint16_t last_message_sent_arb_id; +extern bool last_message_sent_status; +extern uint8_t* last_message_sent_payload; +extern uint8_t last_message_sent_payload_size; + +extern void setup(); + +START_TEST (test_send_single_frame) +{ + fail_if(true); +} +END_TEST + +START_TEST (test_send_multi_frame) +{ + fail_if(true); +} +END_TEST + +Suite* testSuite(void) { + Suite* s = suite_create("iso15765"); + TCase *tc_core = tcase_create("send"); + tcase_add_checked_fixture(tc_core, setup, NULL); + tcase_add_test(tc_core, test_send_single_frame); + tcase_add_test(tc_core, test_send_multi_frame); + suite_add_tcase(s, tc_core); + + return s; +} + +int main(void) { + int numberFailed; + Suite* s = testSuite(); + SRunner *sr = srunner_create(s); + // Don't fork so we can actually use gdb + srunner_set_fork_status(sr, CK_NOFORK); + srunner_run_all(sr, CK_NORMAL); + numberFailed = srunner_ntests_failed(sr); + srunner_free(sr); + return (numberFailed == 0) ? 0 : 1; +} diff --git a/tests/tests.c b/tests/tests.c deleted file mode 100644 index 62b3df8d..00000000 --- a/tests/tests.c +++ /dev/null @@ -1,30 +0,0 @@ -#include -#include -#include - -START_TEST (test_fail) -{ - fail_if(true); -} -END_TEST - -Suite* bitfieldSuite(void) { - Suite* s = suite_create("iso15765"); - TCase *tc_core = tcase_create("core"); - tcase_add_test(tc_core, test_fail); - suite_add_tcase(s, tc_core); - - return s; -} - -int main(void) { - int numberFailed; - Suite* s = bitfieldSuite(); - SRunner *sr = srunner_create(s); - // Don't fork so we can actually use gdb - srunner_set_fork_status(sr, CK_NOFORK); - srunner_run_all(sr, CK_NORMAL); - numberFailed = srunner_ntests_failed(sr); - srunner_free(sr); - return (numberFailed == 0) ? 0 : 1; -} -- 2.16.6