From 72b9b05d828b4cbd51b2b3eda4f5e96166d4515a Mon Sep 17 00:00:00 2001 From: Anton Gerasimov Date: Mon, 18 Dec 2017 10:24:45 +0100 Subject: [PATCH] Minor fixes to make compiler happy --- src/isotp/isotp.c | 30 +++++---------------- src/isotp/isotp.h | 12 +-------- src/isotp/isotp_types.h | 8 +++--- src/isotp/receive.c | 60 ++++++++++++++++++++--------------------- src/isotp/receive.h | 8 +++--- src/isotp/send.c | 23 +++++++++------- src/isotp/send.h | 6 ++--- tests/common.c | 14 ++++++++++ 8 files changed, 75 insertions(+), 86 deletions(-) diff --git a/src/isotp/isotp.c b/src/isotp/isotp.c index ce87f1b..3a02fd1 100644 --- a/src/isotp/isotp.c +++ b/src/isotp/isotp.c @@ -2,31 +2,15 @@ #include #include -/* 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, - frame_padding: ISO_TP_DEFAULT_FRAME_PADDING_STATUS - }; + IsoTpShims shims; + shims.log = log; + shims.send_can_message = send_can_message; + shims.set_timer = set_timer; + shims.frame_padding = ISO_TP_DEFAULT_FRAME_PADDING_STATUS; + return shims; } -void isotp_message_to_string(const IsoTpMessage* message, char* destination, - size_t destination_length) { - snprintf(destination, destination_length, "ID: 0x%" SCNd32 ", Payload: 0x%02x%02x%02x%02x%02x%02x%02x%02x", - message->arbitration_id, - message->payload[0], - message->payload[1], - message->payload[2], - message->payload[3], - message->payload[4], - message->payload[5], - message->payload[6], - message->payload[7]); -} + diff --git a/src/isotp/isotp.h b/src/isotp/isotp.h index 3a3658c..7a7d428 100644 --- a/src/isotp/isotp.h +++ b/src/isotp/isotp.h @@ -24,18 +24,8 @@ IsoTpShims isotp_init_shims(LogShim log, SendCanMessageShim send_can_message, SetTimerShim set_timer); -/* Public: Render an IsoTpMessage as a string into the given buffer. - * - * message - the message to convert to a string, for debug logging. - * destination - the target string buffer. - * destination_length - the size of the destination buffer, i.e. the max size - * for the rendered string. - */ -void isotp_message_to_string(const IsoTpMessage* message, char* destination, - size_t destination_length); - #ifdef __cplusplus } #endif -#endif // __ISOTP_H__ +#endif /* __ISOTP_H__ */ diff --git a/src/isotp/isotp_types.h b/src/isotp/isotp_types.h index 3b7fd26..4d7e1fe 100644 --- a/src/isotp/isotp_types.h +++ b/src/isotp/isotp_types.h @@ -7,9 +7,9 @@ #define CAN_MESSAGE_BYTE_SIZE 8 #define MAX_ISO_TP_MESSAGE_SIZE 4096 -// TODO we want to avoid malloc, and we can't be allocated 4K on the stack for -// each IsoTpMessage, so for now we're setting an artificial max message size -// here - for most multi-frame use cases, 256 bytes is plenty. +/* TODO we want to avoid malloc, and we can't be allocated 4K on the stack for + each IsoTpMessage, so for now we're setting an artificial max message size + here - for most multi-frame use cases, 256 bytes is plenty. */ #define OUR_MAX_ISO_TP_MESSAGE_SIZE 127 /* Private: IsoTp nibble specifics for PCI and Payload. @@ -141,4 +141,4 @@ typedef enum { } #endif -#endif // __ISOTP_TYPES__ +#endif /* __ISOTP_TYPES__ */ diff --git a/src/isotp/receive.c b/src/isotp/receive.c index 35b7a2a..3e043fb 100644 --- a/src/isotp/receive.c +++ b/src/isotp/receive.c @@ -18,7 +18,7 @@ bool isotp_handle_single_frame(IsoTpReceiveHandle* handle, IsoTpMessage* message } bool isotp_handle_multi_frame(IsoTpReceiveHandle* handle, IsoTpMessage* message) { - // call this once all consecutive frames have been received + /* call this once all consecutive frames have been received */ isotp_complete_receive(handle, message); return true; } @@ -27,7 +27,8 @@ bool isotp_send_flow_control_frame(IsoTpShims* shims, IsoTpMessage* message) { uint8_t can_data[CAN_MESSAGE_BYTE_SIZE] = {0}; if(!set_nibble(PCI_NIBBLE_INDEX, PCI_FLOW_CONTROL_FRAME, can_data, sizeof(can_data))) { - shims->log("Unable to set PCI in CAN data"); + if(shims->log) + shims->log("Unable to set PCI in CAN data"); return false; } @@ -39,12 +40,11 @@ bool isotp_send_flow_control_frame(IsoTpShims* shims, IsoTpMessage* message) { IsoTpReceiveHandle isotp_receive(IsoTpShims* shims, const uint32_t arbitration_id, IsoTpMessageReceivedHandler callback) { - IsoTpReceiveHandle handle = { - success: false, - completed: false, - arbitration_id: arbitration_id, - message_received_callback: callback - }; + IsoTpReceiveHandle handle; + handle.success = false; + handle.completed = false; + handle.arbitration_id = arbitration_id; + handle.message_received_callback = callback; return handle; } @@ -52,13 +52,7 @@ IsoTpReceiveHandle isotp_receive(IsoTpShims* shims, IsoTpMessage isotp_continue_receive(IsoTpShims* shims, IsoTpReceiveHandle* handle, const uint32_t arbitration_id, const uint8_t data[], const uint8_t size) { - IsoTpMessage message = { - arbitration_id: arbitration_id, - completed: false, - multi_frame: false, - payload: {0}, - size: 0 - }; + IsoTpMessage message = {arbitration_id, {0}, 0, false, false}; if(size < 1) { return message; @@ -66,10 +60,10 @@ IsoTpMessage isotp_continue_receive(IsoTpShims* shims, if(handle->arbitration_id != arbitration_id) { if(shims->log != NULL) { - // You may turn this on for debugging, but in normal operation it's - // very noisy if you are passing all received CAN messages to this - // handler. - /* shims->log("The arb ID 0x%x doesn't match the expected rx ID 0x%x", */ + /* You may turn this on for debugging, but in normal operation it's + very noisy if you are passing all received CAN messages to this + handler. + shims->log("The arb ID 0x%x doesn't match the expected rx ID 0x%x", */ /* arbitration_id, handle->arbitration_id); */ } return message; @@ -78,9 +72,9 @@ IsoTpMessage isotp_continue_receive(IsoTpShims* shims, IsoTpProtocolControlInformation pci = (IsoTpProtocolControlInformation) get_nibble(data, size, 0); - // 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 + /* 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: { @@ -97,24 +91,26 @@ IsoTpMessage isotp_continue_receive(IsoTpShims* shims, isotp_handle_single_frame(handle, &message); break; } - //If multi-frame, then the payload length is contained in the 12 - //bits following the first nibble of Byte 0. + /* If multi-frame, then the payload length is contained in the 12 + bits following the first nibble of Byte 0. */ case PCI_FIRST_FRAME: { uint16_t payload_length = (get_nibble(data, size, 1) << 8) + get_byte(data, size, 1); if(payload_length > OUR_MAX_ISO_TP_MESSAGE_SIZE) { - shims->log("Multi-frame response too large for receive buffer."); + if(shims->log) + shims->log("Multi-frame response too large for receive buffer."); break; } - //Need to allocate memory for the combination of multi-frame - //messages. That way we don't have to allocate 4k of memory - //for each multi-frame response. + /* Need to allocate memory for the combination of multi-frame + messages. That way we don't have to allocate 4k of memory + for each multi-frame response. */ uint8_t* combined_payload = NULL; combined_payload = (uint8_t*)malloc(sizeof(uint8_t)*payload_length); if(combined_payload == NULL) { - shims->log("Unable to allocate memory for multi-frame response."); + if(shims->log) + shims->log("Unable to allocate memory for multi-frame response."); break; } @@ -144,13 +140,15 @@ IsoTpMessage isotp_continue_receive(IsoTpShims* shims, if(handle->received_buffer_size != handle->incoming_message_size){ free(handle->receive_buffer); handle->success = false; - shims->log("Error capturing all bytes of multi-frame. Freeing memory."); + if(shims->log) + shims->log("Error capturing all bytes of multi-frame. Freeing memory."); } else { memcpy(message.payload,&handle->receive_buffer[0],handle->incoming_message_size); free(handle->receive_buffer); message.size = handle->incoming_message_size; message.completed = true; - shims->log("Successfully captured all of multi-frame. Freeing memory."); + if(shims->log) + shims->log("Successfully captured all of multi-frame. Freeing memory."); handle->success = true; handle->completed = true; diff --git a/src/isotp/receive.h b/src/isotp/receive.h index 6788914..7d7a893 100644 --- a/src/isotp/receive.h +++ b/src/isotp/receive.h @@ -24,15 +24,15 @@ typedef struct { bool completed; bool success; - // Private + /* Private */ uint32_t arbitration_id; IsoTpMessageReceivedHandler message_received_callback; uint16_t timeout_ms; - // timeout_ms: ISO_TP_DEFAULT_RESPONSE_TIMEOUT, + /* timeout_ms: ISO_TP_DEFAULT_RESPONSE_TIMEOUT, */ uint8_t* receive_buffer; uint16_t received_buffer_size; uint16_t incoming_message_size; - // TODO timer callback for multi frame + /* TODO timer callback for multi frame */ } IsoTpReceiveHandle; /* Public: Initiate receiving a single ISO-TP message on a particular @@ -83,4 +83,4 @@ IsoTpMessage isotp_continue_receive(IsoTpShims* shims, } #endif -#endif // __ISOTP_RECEIVE_H__ +#endif /* __ISOTP_RECEIVE_H__ */ diff --git a/src/isotp/send.c b/src/isotp/send.c index e849bb2..68c231d 100644 --- a/src/isotp/send.c +++ b/src/isotp/send.c @@ -22,13 +22,15 @@ IsoTpSendHandle isotp_send_single_frame(IsoTpShims* shims, IsoTpMessage* message uint8_t can_data[CAN_MESSAGE_BYTE_SIZE] = {0}; if(!set_nibble(PCI_NIBBLE_INDEX, PCI_SINGLE, can_data, sizeof(can_data))) { - shims->log("Unable to set PCI in CAN data"); + if(shims->log) + 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))) { - shims->log("Unable to set payload length in CAN data"); + if(shims->log) + shims->log("Unable to set payload length in CAN data"); return handle; } @@ -45,14 +47,15 @@ IsoTpSendHandle isotp_send_single_frame(IsoTpShims* shims, IsoTpMessage* message IsoTpSendHandle isotp_send_multi_frame(IsoTpShims* shims, IsoTpMessage* message, IsoTpMessageSentHandler callback) { - // TODO make sure to copy message into a local buffer - shims->log("Only single frame messages are supported"); + /* TODO make sure to copy message into a local buffer */ + if(shims->log) + shims->log("Only single frame messages are supported"); IsoTpSendHandle handle = { success: false, completed: true }; - // TODO need to set sending and receiving arbitration IDs separately if we - // can't always just add 0x8 (and I think we can't) + /* TODO need to set sending and receiving arbitration IDs separately if we + can't always just add 0x8 (and I think we can't) */ return handle; } @@ -75,11 +78,11 @@ IsoTpSendHandle isotp_send(IsoTpShims* shims, const uint16_t arbitration_id, bool isotp_continue_send(IsoTpShims* shims, IsoTpSendHandle* handle, const uint16_t arbitration_id, const uint8_t data[], const uint8_t size) { - // TODO this will need to be tested when we add multi-frame support, - // which is when it'll be necessary to pass in CAN messages to SENDING - // handles. + /* TODO this will need to be tested when we add multi-frame support, + which is when it'll be necessary to pass in CAN messages to SENDING + handles. */ if(handle->receiving_arbitration_id != arbitration_id) { - if(shims->log != NULL) { + if(shims->log) { shims->log("The arb ID 0x%x doesn't match the expected tx continuation ID 0x%x", arbitration_id, handle->receiving_arbitration_id); } diff --git a/src/isotp/send.h b/src/isotp/send.h index 1af3266..9e072d5 100644 --- a/src/isotp/send.h +++ b/src/isotp/send.h @@ -25,12 +25,12 @@ typedef struct { bool completed; bool success; - // Private + /* Private */ 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 + /* TODO going to need some state here for multi frame messages */ } IsoTpSendHandle; /* Public: Initiate sending a single ISO-TP message. @@ -83,4 +83,4 @@ bool isotp_continue_send(IsoTpShims* shims, IsoTpSendHandle* handle, } #endif -#endif // __ISOTP_SEND_H__ +#endif /* __ISOTP_SEND_H__ */ diff --git a/tests/common.c b/tests/common.c index a9eed39..e6ecf4c 100644 --- a/tests/common.c +++ b/tests/common.c @@ -23,6 +23,20 @@ bool last_message_sent_status; uint8_t last_message_sent_payload[OUR_MAX_ISO_TP_MESSAGE_SIZE]; uint8_t last_message_sent_payload_size; +void isotp_message_to_string(const IsoTpMessage* message, char* destination, + size_t destination_length) { + snprintf(destination, destination_length, "ID: 0x%" SCNd32 ", Payload: 0x%02x%02x%02x%02x%02x%02x%02x%02x", + message->arbitration_id, + message->payload[0], + message->payload[1], + message->payload[2], + message->payload[3], + message->payload[4], + message->payload[5], + message->payload[6], + message->payload[7]); +} + void debug(const char* format, ...) { va_list args; va_start(args, format);