From 10a35b0a7c380d77cdd24ac90d6aa0abd4601f3e Mon Sep 17 00:00:00 2001 From: Christopher Peplin Date: Tue, 31 Dec 2013 12:59:04 -0500 Subject: [PATCH] Draft reworking of API. --- src/isotp/isotp.c | 76 ++++++++++++++++++++++++++++++++++++++-------------- src/isotp/isotp.h | 44 +++++++++++++++++++++--------- src/isotp/receive.c | 47 +++++++++++--------------------- src/isotp/receive.h | 10 +++---- src/isotp/send.c | 54 +++++++++++++++++++++++-------------- src/isotp/send.h | 6 ++--- tests/common.c | 6 ++--- tests/test_receive.c | 10 +++---- tests/test_send.c | 30 +++++++++++++-------- 9 files changed, 173 insertions(+), 110 deletions(-) diff --git a/src/isotp/isotp.c b/src/isotp/isotp.c index f303e214..f5b20795 100644 --- a/src/isotp/isotp.c +++ b/src/isotp/isotp.c @@ -7,10 +7,14 @@ 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; +// TODO why isn't this picked up from the header? +extern IsoTpHandle isotp_receive(IsoTpShims* shims, const uint16_t arbitration_id, + IsoTpMessageReceivedHandler callback); -void isotp_set_timeout(IsoTpHandler* handler, uint16_t timeout_ms) { - handler->timeout_ms = timeout_ms; -} + +/* 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) { @@ -22,26 +26,58 @@ IsoTpShims isotp_init_shims(LogShim log, SendCanMessageShim send_can_message, 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, - arbitration_id: arbitration_id, - 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; -} - void isotp_message_to_string(const IsoTpMessage* message, char* destination, size_t destination_length) { snprintf(destination, destination_length,"ID: 0x%02x, Payload: 0x%llx", // TODO the payload may be backwards here message->arbitration_id, message->payload); } + +void isotp_receive_can_frame(IsoTpShims* shims, IsoTpHandle* handle, + const uint16_t arbitration_id, const uint8_t data[], + const uint8_t data_length) { + if(data_length < 1) { + return; + } + + if(handle->type == ISOTP_HANDLE_RECEIVING) { + if(handle->receive_handle.arbitration_id != arbitration_id) { + return; + } + } else if(handle->type == ISOTP_HANDLE_SENDING) { + if(handle->send_handle.receiving_arbitration_id != arbitration_id) { + return; + } + } else { + shims->log("The ISO-TP handle is corrupt"); + } + + IsoTpProtocolControlInformation pci = (IsoTpProtocolControlInformation) + get_nibble(data, data_length, 0); + + uint8_t payload_length = get_nibble(data, data_length, 1); + uint8_t payload[payload_length]; + if(payload_length > 0 && data_length > 0) { + memcpy(payload, &data[1], payload_length); + } + + // TODO this is set up to handle rx a response with a payload, but not to + // handle flow control responses for multi frame messages that we're in the + // process of sending + + switch(pci) { + case PCI_SINGLE: { + IsoTpMessage message = { + arbitration_id: arbitration_id, + payload: payload, + size: payload_length + }; + + isotp_handle_single_frame(handle, &message); + break; + } + default: + shims->log("Only single frame messages are supported"); + break; + } +} diff --git a/src/isotp/isotp.h b/src/isotp/isotp.h index 74a4897a..10352383 100644 --- a/src/isotp/isotp.h +++ b/src/isotp/isotp.h @@ -39,21 +39,42 @@ typedef struct { } IsoTpShims; typedef struct { - IsoTpShims* shims; uint16_t arbitration_id; IsoTpMessageReceivedHandler message_received_callback; - IsoTpMessageSentHandler message_sent_callback; - IsoTpCanFrameSentHandler can_frame_sent_callback; // Private uint16_t timeout_ms; + // timeout_ms: ISO_TP_DEFAULT_RESPONSE_TIMEOUT, bool frame_padding; + // frame_padding: ISO_TP_DEFAULT_FRAME_PADDING_STATUS, uint8_t* receive_buffer; uint16_t received_buffer_size; uint16_t incoming_message_size; - bool sending; // TODO timer callback -} IsoTpHandler; +} IsoTpReceiveHandle; + +typedef struct { + uint16_t sending_arbitration_id; + uint16_t receiving_arbitration_id; + IsoTpMessageSentHandler message_sent_callback; + IsoTpCanFrameSentHandler can_frame_sent_callback; + + // TODO going to need some state here for multi frame messages +} IsoTpSendHandle; + +typedef enum { + ISOTP_HANDLE_SENDING, + ISOTP_HANDLE_RECEIVING +} IsoTpHandleType; + +typedef struct { + bool success; + bool completed; + IsoTpHandleType type; + IsoTpReceiveHandle receive_handle; + IsoTpSendHandle send_handle; +} IsoTpHandle; + typedef enum { PCI_SINGLE = 0x0, @@ -72,11 +93,9 @@ IsoTpShims isotp_init_shims(LogShim log, SendCanMessageShim send_can_message, SetTimerShim set_timer); -IsoTpHandler isotp_init(IsoTpShims* shims, - uint16_t arbitration_id, - IsoTpMessageReceivedHandler message_received_callback, - IsoTpMessageSentHandler message_sent_callback, - IsoTpCanFrameSentHandler can_frame_sent_callback); +void isotp_receive_can_frame(IsoTpShims* shims, IsoTpHandle* handle, + const uint16_t arbitration_id, const uint8_t data[], + const uint8_t size); /* Public: Change the timeout for waiting on an ISO-TP response frame. * @@ -85,13 +104,14 @@ IsoTpHandler isotp_init(IsoTpShims* shims, * handler - the ISO-TP handler to modify. * timeout - the new timeout in milliseconds. */ -void isotp_set_timeout(IsoTpHandler* handler, uint16_t timeout_ms); +// void isotp_set_timeout(IsoTpHandler* handler, uint16_t timeout_ms); -void isotp_destroy(IsoTpHandler* handler); +// void isotp_destroy(IsoTpHandler* handler); void isotp_message_to_string(const IsoTpMessage* message, char* destination, size_t destination_length); + #ifdef __cplusplus } #endif diff --git a/src/isotp/receive.c b/src/isotp/receive.c index bd94e7aa..1d623384 100644 --- a/src/isotp/receive.c +++ b/src/isotp/receive.c @@ -1,42 +1,27 @@ #include -void isotp_handle_single_frame(IsoTpHandler* handler, IsoTpMessage* message) { - isotp_complete_receive(handler, message); +void isotp_handle_single_frame(IsoTpHandle* handle, + IsoTpMessage* message) { + isotp_complete_receive(handle, message); } -void isotp_complete_receive(IsoTpHandler* handler, IsoTpMessage* message) { - handler->message_received_callback(message); +void isotp_complete_receive(IsoTpHandle* handle, IsoTpMessage* message) { + handle->receive_handle.message_received_callback(message); } -void isotp_receive_can_frame(IsoTpHandler* handler, - const uint16_t arbitration_id, const uint8_t data[], - const uint8_t data_length) { - if(arbitration_id != handler->arbitration_id || data_length < 1) { - return; - } - - IsoTpProtocolControlInformation pci = (IsoTpProtocolControlInformation) - get_nibble(data, data_length, 0); - - uint8_t payload_length = get_nibble(data, data_length, 1); - uint8_t payload[payload_length]; - if(payload_length > 0 && data_length > 0) { - memcpy(payload, &data[1], payload_length); - } - - IsoTpMessage message = { +IsoTpHandle isotp_receive(IsoTpShims* shims, + const uint16_t arbitration_id, IsoTpMessageReceivedHandler callback) { + IsoTpReceiveHandle receive_handle = { arbitration_id: arbitration_id, - payload: payload, - size: payload_length + message_received_callback: callback }; - switch(pci) { - case PCI_SINGLE: - isotp_handle_single_frame(handler, &message); - break; - default: - handler->shims->log("Only single frame messages are supported"); - break; - } + IsoTpHandle handle = { + success: false, + completed: true, + receive_handle: receive_handle, + type: ISOTP_HANDLE_RECEIVING + }; + return handle; } diff --git a/src/isotp/receive.h b/src/isotp/receive.h index feca993c..3eee89e2 100644 --- a/src/isotp/receive.h +++ b/src/isotp/receive.h @@ -9,13 +9,13 @@ extern "C" { #endif -void isotp_handle_single_frame(IsoTpHandler* handler, IsoTpMessage* message); +void isotp_complete_receive(IsoTpHandle* handle, IsoTpMessage* message); -void isotp_complete_receive(IsoTpHandler* handler, IsoTpMessage* message); +void isotp_handle_single_frame(IsoTpHandle* handle, + IsoTpMessage* message); -void isotp_receive_can_frame(IsoTpHandler* handler, - const uint16_t arbitration_id, const uint8_t data[], - const uint8_t size); +IsoTpHandle isotp_receive(IsoTpShims* shims, const uint16_t arbitration_id, + IsoTpMessageReceivedHandler callback); #ifdef __cplusplus } diff --git a/src/isotp/send.c b/src/isotp/send.c index ae208eb3..67703cae 100644 --- a/src/isotp/send.c +++ b/src/isotp/send.c @@ -4,52 +4,66 @@ #define PAYLOAD_LENGTH_NIBBLE_INDEX 1 #define PAYLOAD_BYTE_INDEX 1 -void isotp_complete_send(IsoTpHandler* handler, IsoTpMessage* message, - bool status) { - handler->message_sent_callback(message, status); +void isotp_complete_send(IsoTpShims* shims, IsoTpMessage* message, + bool status, IsoTpMessageSentHandler callback) { + callback(message, status); } -bool isotp_send_single_frame(IsoTpHandler* handler, IsoTpMessage* message) { +IsoTpHandle isotp_send_single_frame(IsoTpShims* shims, IsoTpMessage* message, + IsoTpMessageSentHandler callback) { + IsoTpHandle handle = { + success: false, + completed: true, + type: ISOTP_HANDLE_SENDING + }; + uint8_t can_data[CAN_MESSAGE_BYTE_SIZE] = {0}; if(!set_nibble(PCI_NIBBLE_INDEX, PCI_SINGLE, can_data, sizeof(can_data))) { - handler->shims->log("Unable to set PCI in CAN data"); - return false; + shims->log("Unable to set PCI in CAN data"); + return handle; } if(!set_nibble(PAYLOAD_LENGTH_NIBBLE_INDEX, message->size, can_data, sizeof(can_data))) { - handler->shims->log("Unable to set payload length in CAN data"); - return false; + shims->log("Unable to set payload length in CAN data"); + return handle; } if(message->size > 0) { memcpy(&can_data[1], message->payload, message->size); } - handler->shims->send_can_message(message->arbitration_id, can_data, + shims->send_can_message(message->arbitration_id, can_data, 1 + message->size); - isotp_complete_send(handler, message, true); - return true; + handle.success = true; + isotp_complete_send(shims, message, true, callback); + return handle; } -bool isotp_send_multi_frame(IsoTpHandler* handler, IsoTpMessage* message) { +IsoTpHandle isotp_send_multi_frame(IsoTpShims* shims, IsoTpMessage* message, + IsoTpMessageSentHandler callback) { // TODO make sure to copy payload into a local buffer - handler->shims->log("Only single frame messages are supported"); - return false; + shims->log("Only single frame messages are supported"); + IsoTpHandle handle = { + success: false, + completed: true, + type: ISOTP_HANDLE_SENDING + }; + return handle; } -bool isotp_send(IsoTpHandler* handler, const uint8_t* payload, - uint16_t size) { - // we determine if it's single/multi frame and start the send +IsoTpHandle isotp_send(IsoTpShims* shims, const uint16_t arbitration_id, + const uint8_t* payload, uint16_t size, + IsoTpMessageSentHandler callback) { IsoTpMessage message = { - arbitration_id: handler->arbitration_id, + arbitration_id: arbitration_id, payload: payload, size: size }; if(size < 8) { - return isotp_send_single_frame(handler, &message); + return isotp_send_single_frame(shims, &message, callback); } else { - return isotp_send_multi_frame(handler, &message); + return isotp_send_multi_frame(shims, &message, callback); } } diff --git a/src/isotp/send.h b/src/isotp/send.h index 2031621b..937e5320 100644 --- a/src/isotp/send.h +++ b/src/isotp/send.h @@ -9,9 +9,9 @@ extern "C" { #endif -// TODO I don't love this API -bool isotp_send(IsoTpHandler* handler, const uint8_t* payload, - uint16_t payload_size); +IsoTpHandle isotp_send(IsoTpShims* shims, const uint16_t arbitration_id, + const uint8_t* payload, uint16_t size, + IsoTpMessageSentHandler callback); #ifdef __cplusplus } diff --git a/tests/common.c b/tests/common.c index 833a5ed4..4ad8eff1 100644 --- a/tests/common.c +++ b/tests/common.c @@ -5,7 +5,7 @@ #include IsoTpShims SHIMS; -IsoTpHandler ISOTP_HANDLER; +IsoTpHandle HANDLE; uint16_t last_can_frame_sent_arb_id; uint8_t last_can_payload_sent[8]; @@ -83,8 +83,8 @@ void can_frame_sent(const uint16_t arbitration_id, const uint8_t* payload, 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); + // TODO + /* HANDLE = isotp_receive(&SHIMS, 0x2a, message_received); */ last_message_sent_payload = malloc(MAX_ISO_TP_MESSAGE_SIZE); last_message_received_payload = malloc(MAX_ISO_TP_MESSAGE_SIZE); memset(last_can_payload_sent, 0, sizeof(last_can_payload_sent)); diff --git a/tests/test_receive.c b/tests/test_receive.c index b116dd23..17df9046 100644 --- a/tests/test_receive.c +++ b/tests/test_receive.c @@ -6,7 +6,7 @@ #include extern IsoTpShims SHIMS; -extern IsoTpHandler ISOTP_HANDLER; +extern IsoTpHandle HANDLE; extern uint16_t last_can_frame_sent_arb_id; extern uint8_t last_can_payload_sent; @@ -28,7 +28,7 @@ extern void setup(); START_TEST (test_receive_wrong_id) { const uint8_t data[CAN_MESSAGE_BYTE_SIZE] = {0}; - isotp_receive_can_frame(&ISOTP_HANDLER, 0x100, data, 1); + isotp_receive_can_frame(&SHIMS, &HANDLE, 0x100, data, 1); fail_if(message_was_received); } END_TEST @@ -37,7 +37,7 @@ START_TEST (test_receive_bad_pci) { // 4 is a reserved number for the PCI field - only 0-3 are allowed const uint8_t data[CAN_MESSAGE_BYTE_SIZE] = {0x40}; - isotp_receive_can_frame(&ISOTP_HANDLER, 0x2a, data, 1); + isotp_receive_can_frame(&SHIMS, &HANDLE, 0x2a, data, 1); fail_if(message_was_received); } END_TEST @@ -45,7 +45,7 @@ END_TEST START_TEST (test_receive_single_frame_empty_payload) { const uint8_t data[CAN_MESSAGE_BYTE_SIZE] = {0x00, 0x12, 0x34}; - isotp_receive_can_frame(&ISOTP_HANDLER, 0x2a, data, 3); + isotp_receive_can_frame(&SHIMS, &HANDLE, 0x2a, data, 3); fail_unless(message_was_received); ck_assert_int_eq(last_message_received_arb_id, 0x2a); ck_assert_int_eq(last_message_received_payload_size, 0); @@ -55,7 +55,7 @@ END_TEST START_TEST (test_receive_single_frame) { const uint8_t data[CAN_MESSAGE_BYTE_SIZE] = {0x02, 0x12, 0x34}; - isotp_receive_can_frame(&ISOTP_HANDLER, 0x2a, data, 3); + isotp_receive_can_frame(&SHIMS, &HANDLE, 0x2a, data, 3); 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); diff --git a/tests/test_send.c b/tests/test_send.c index f5037536..599552ae 100644 --- a/tests/test_send.c +++ b/tests/test_send.c @@ -6,7 +6,9 @@ #include extern IsoTpShims SHIMS; -extern IsoTpHandler ISOTP_HANDLER; +extern IsoTpHandle HANDLE; + +extern void message_sent(const IsoTpMessage* message, const bool success); extern uint16_t last_can_frame_sent_arb_id; extern uint8_t last_can_payload_sent[8]; @@ -27,13 +29,16 @@ extern void setup(); START_TEST (test_send_empty_payload) { - fail_unless(isotp_send(&ISOTP_HANDLER, NULL, 0)); - ck_assert_int_eq(last_message_sent_arb_id, ISOTP_HANDLER.arbitration_id); + uint16_t arbitration_id = 0x2a; + IsoTpHandle foo = isotp_send(&SHIMS, arbitration_id, NULL, 0, message_sent); + fail_unless(handle.success); + fail_unless(handle.completed); + ck_assert_int_eq(last_message_sent_arb_id, arbitration_id); fail_unless(last_message_sent_status); ck_assert_int_eq(last_message_sent_payload[0], NULL); ck_assert_int_eq(last_message_sent_payload_size, 0); - ck_assert_int_eq(last_can_frame_sent_arb_id, ISOTP_HANDLER.arbitration_id); + ck_assert_int_eq(last_can_frame_sent_arb_id, arbitration_id); fail_unless(can_frame_was_sent); ck_assert_int_eq(last_can_payload_sent[0], 0x0); ck_assert_int_eq(last_can_payload_size, 1); @@ -43,14 +48,16 @@ END_TEST START_TEST (test_send_single_frame) { const uint8_t payload[] = {0x12, 0x34}; - fail_unless(isotp_send(&ISOTP_HANDLER, &payload, sizeof(payload))); - ck_assert_int_eq(last_message_sent_arb_id, ISOTP_HANDLER.arbitration_id); + uint16_t arbitration_id = 0x2a; + IsoTpHandle handle = isotp_send(&SHIMS, arbitration_id, payload, sizeof(payload), + message_sent); + ck_assert_int_eq(last_message_sent_arb_id, arbitration_id); fail_unless(last_message_sent_status); ck_assert_int_eq(last_message_sent_payload[0], 0x12); ck_assert_int_eq(last_message_sent_payload[1], 0x34); ck_assert_int_eq(last_message_sent_payload_size, 2); - ck_assert_int_eq(last_can_frame_sent_arb_id, ISOTP_HANDLER.arbitration_id); + ck_assert_int_eq(last_can_frame_sent_arb_id, arbitration_id); fail_unless(can_frame_was_sent); ck_assert_int_eq(last_can_payload_sent[0], 0x2); ck_assert_int_eq(last_can_payload_sent[1], 0x12); @@ -63,10 +70,11 @@ START_TEST (test_send_multi_frame) { const uint8_t payload[] = {0x12, 0x34, 0x56, 0x78, 0x90, 0x01, 0x23, 0x45, 0x67, 0x89}; - bool status = isotp_send(&ISOTP_HANDLER, &payload, sizeof(payload)); - fail_if(status); - fail_if(last_message_sent_status); - fail_if(can_frame_was_sent); + uint16_t arbitration_id = 0x2a; + IsoTpHandle handle = isotp_send(&SHIMS, arbitration_id, payload, sizeof(payload), + message_sent); + fail_unless(handle.completed); + fail_if(handle.success); } END_TEST -- 2.16.6