Split up functions to rx CAN messages for rx and tx of ISO-TP.

9 files changed, 80 insertions, 95 deletions

diff --git a/README.mkd b/README.mkd
index 1e2c29aa..6c3efa94 100644
--- a/README.mkd
+++ b/README.mkd
@@ -58,7 +58,7 @@ With your shims in hand, send an ISO-TP message:
         // You received the message! Do something with it.
     }
 
-    IsoTpHandle handle = isotp_send(&shims, 0x100, NULL, 0, message_sent);
+    IsoTpSendHandle handle = isotp_send(&shims, 0x100, NULL, 0, message_sent);
 
     if(handle.completed) {
         if(!handle.success) {
@@ -72,7 +72,10 @@ With your shims in hand, send an ISO-TP message:
     } else {
         while(true) {
             // Continue to read from CAN, passing off each message to the handle
-            bool complete = isotp_receive_can_frame(&shims, &handle, 0x100, data, size);
+            // this will return true when the message is completely sent (which
+            // may take more than one call if it was multi frame and we're waiting
+            // on flow control responses from the receiver)
+            bool complete = isotp_continue_send(&shims, &handle, 0x100, data, size);
 
             if(complete && handle.completed) {
                 if(handle.success) {
@@ -90,19 +93,19 @@ Finally, receive an ISO-TP message:
 
     // Optional: This is your callback for when a complete ISO-TP message is
     // received at the arbitration ID you specify. The completed message is
-    // also returned by isotp_receive_can_frame, which can sometimes be more
+    // also returned by isotp_continue_receive, which can sometimes be more
     // useful since you have more context.
     void message_received(const IsoTpMessage* message) {
     }
 
-    IsoTpHandle handle = isotp_receive(&shims, 0x100, message_received);
+    IsoTpReceiveHandle handle = isotp_receive(&shims, 0x100, message_received);
     if(!handle.success) {
         // something happened and it already failed - possibly we aren't able to
         // send CAN messages
     } else {
         while(true) {
             // Continue to read from CAN, passing off each message to the handle
-            IsoTp message = isotp_receive_can_frame(&shims, &handle, 0x100, data, size);
+            IsoTpMessage message = isotp_continue_receive(&shims, &handle, 0x100, data, size);
 
             if(message.completed && handle.completed) {
                 if(handle.success) {
diff --git a/src/isotp/isotp.c b/src/isotp/isotp.c
index e233d386..6de40c31 100644
--- a/src/isotp/isotp.c
+++ b/src/isotp/isotp.c
@@ -33,9 +33,9 @@ void isotp_message_to_string(const IsoTpMessage* message, char* destination,
             message->payload[7]);
 }
 
-IsoTpMessage isotp_receive_can_frame(IsoTpShims* shims, IsoTpHandle* handle,
-        const uint16_t arbitration_id, const uint8_t data[],
-        const uint8_t data_length) {
+IsoTpMessage isotp_continue_receive(IsoTpShims* shims,
+        IsoTpReceiveHandle* handle, const uint16_t arbitration_id,
+        const uint8_t data[], const uint8_t size) {
     IsoTpMessage message = {
         arbitration_id: arbitration_id,
         completed: false,
@@ -43,40 +43,24 @@ IsoTpMessage isotp_receive_can_frame(IsoTpShims* shims, IsoTpHandle* handle,
         size: 0
     };
 
-    if(data_length < 1) {
+    if(size < 1) {
         return message;
     }
 
-    if(handle->type == ISOTP_HANDLE_RECEIVING) {
-        if(handle->receive_handle.arbitration_id != arbitration_id) {
-            if(shims->log != NULL)  {
-                shims->log("The arb ID 0x%x doesn't match the expected rx ID 0x%x",
-                        arbitration_id, handle->receive_handle.arbitration_id);
-            }
-            return message;
+    if(handle->arbitration_id != arbitration_id) {
+        if(shims->log != NULL)  {
+            shims->log("The arb ID 0x%x doesn't match the expected rx ID 0x%x",
+                    arbitration_id, handle->arbitration_id);
         }
-    } else if(handle->type == ISOTP_HANDLE_SENDING) {
-        // 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->send_handle.receiving_arbitration_id != arbitration_id) {
-            if(shims->log != NULL) {
-                shims->log("The arb ID 0x%x doesn't match the expected tx continuation ID 0x%x",
-                        arbitration_id, handle->send_handle.receiving_arbitration_id);
-            }
-            return message;
-        }
-    } else {
-        shims->log("The ISO-TP handle is corrupt");
         return message;
     }
 
     IsoTpProtocolControlInformation pci = (IsoTpProtocolControlInformation)
-            get_nibble(data, data_length, 0);
+            get_nibble(data, size, 0);
 
-    uint8_t payload_length = get_nibble(data, data_length, 1);
+    uint8_t payload_length = get_nibble(data, size, 1);
     uint8_t payload[payload_length];
-    if(payload_length > 0 && data_length > 0) {
+    if(payload_length > 0 && size > 0) {
         memcpy(payload, &data[1], payload_length);
     }
 
@@ -102,3 +86,20 @@ IsoTpMessage isotp_receive_can_frame(IsoTpShims* shims, IsoTpHandle* handle,
     }
     return message;
 }
+
+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.
+    if(handle->receiving_arbitration_id != arbitration_id) {
+        if(shims->log != NULL) {
+            shims->log("The arb ID 0x%x doesn't match the expected tx continuation ID 0x%x",
+                    arbitration_id, handle->receiving_arbitration_id);
+        }
+        return false;
+    }
+    return false;
+}
+
diff --git a/src/isotp/isotp.h b/src/isotp/isotp.h
index adf0f247..f4ffa371 100644
--- a/src/isotp/isotp.h
+++ b/src/isotp/isotp.h
@@ -43,6 +43,8 @@ typedef struct {
 } IsoTpShims;
 
 typedef struct {
+    bool success;
+    bool completed;
     uint16_t arbitration_id;
     IsoTpMessageReceivedHandler message_received_callback;
 
@@ -54,15 +56,16 @@ typedef struct {
     uint8_t* receive_buffer;
     uint16_t received_buffer_size;
     uint16_t incoming_message_size;
-    // TODO timer callback
+    // TODO timer callback for multi frame
 } IsoTpReceiveHandle;
 
 typedef struct {
+    bool success;
+    bool completed;
     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;
 
@@ -71,15 +74,6 @@ typedef enum {
     ISOTP_HANDLE_RECEIVING
 } IsoTpHandleType;
 
-typedef struct {
-    bool success;
-    bool completed;
-    IsoTpHandleType type;
-    IsoTpReceiveHandle receive_handle;
-    IsoTpSendHandle send_handle;
-} IsoTpHandle;
-
-
 typedef enum {
     PCI_SINGLE = 0x0,
     PCI_FIRST_FRAME = 0x1,
@@ -97,18 +91,14 @@ IsoTpShims isotp_init_shims(LogShim log,
         SendCanMessageShim send_can_message,
         SetTimerShim set_timer);
 
-/* Public:
- *
- */
-IsoTpMessage isotp_receive_can_frame(IsoTpShims* shims, IsoTpHandle* handle,
+IsoTpMessage isotp_continue_receive(IsoTpShims* shims,
+        IsoTpReceiveHandle* handle, const uint16_t arbitration_id,
+        const uint8_t data[], const uint8_t size);
+
+bool isotp_continue_send(IsoTpShims* shims, IsoTpSendHandle* handle,
         const uint16_t arbitration_id, const uint8_t data[],
         const uint8_t size);
 
-// TODO perhaps this makes more sense as 2 functions:
-// bool isotp_continue_send()
-// IsoTpMessage isotp_continue_receive()
-// but both with the same args
-
 /* 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.
@@ -118,16 +108,14 @@ IsoTpMessage isotp_receive_can_frame(IsoTpShims* shims, IsoTpHandle* handle,
  */
 // void isotp_set_timeout(IsoTpHandler* handler, uint16_t timeout_ms);
 
-// void isotp_destroy(IsoTpHandler* handler);
-
 void isotp_message_to_string(const IsoTpMessage* message, char* destination,
         size_t destination_length);
 
-IsoTpHandle isotp_send(IsoTpShims* shims, const uint16_t arbitration_id,
+IsoTpSendHandle isotp_send(IsoTpShims* shims, const uint16_t arbitration_id,
         const uint8_t payload[], uint16_t size,
         IsoTpMessageSentHandler callback);
 
-IsoTpHandle isotp_receive(IsoTpShims* shims,
+IsoTpReceiveHandle isotp_receive(IsoTpShims* shims,
         const uint16_t arbitration_id, IsoTpMessageReceivedHandler callback);
 
 #ifdef __cplusplus
diff --git a/src/isotp/receive.c b/src/isotp/receive.c
index b07950c5..3eba9b53 100644
--- a/src/isotp/receive.c
+++ b/src/isotp/receive.c
@@ -1,28 +1,24 @@
 #include <isotp/receive.h>
 
-bool isotp_handle_single_frame(IsoTpHandle* handle, IsoTpMessage* message) {
+bool isotp_handle_single_frame(IsoTpReceiveHandle* handle, IsoTpMessage* message) {
     isotp_complete_receive(handle, message);
     return true;
 }
 
-void isotp_complete_receive(IsoTpHandle* handle, IsoTpMessage* message) {
-    if(handle->receive_handle.message_received_callback != NULL) {
-        handle->receive_handle.message_received_callback(message);
+void isotp_complete_receive(IsoTpReceiveHandle* handle, IsoTpMessage* message) {
+    if(handle->message_received_callback != NULL) {
+        handle->message_received_callback(message);
     }
 }
 
-IsoTpHandle isotp_receive(IsoTpShims* shims,
+IsoTpReceiveHandle isotp_receive(IsoTpShims* shims,
         const uint16_t arbitration_id, IsoTpMessageReceivedHandler callback) {
-    IsoTpReceiveHandle receive_handle = {
+    IsoTpReceiveHandle handle = {
+        success: false,
+        completed: false,
         arbitration_id: arbitration_id,
         message_received_callback: callback
     };
 
-    IsoTpHandle handle = {
-        success: false,
-        completed: false,
-        receive_handle: receive_handle,
-        type: ISOTP_HANDLE_RECEIVING
-    };
     return handle;
 }
diff --git a/src/isotp/receive.h b/src/isotp/receive.h
index b01e12de..f43d32e2 100644
--- a/src/isotp/receive.h
+++ b/src/isotp/receive.h
@@ -9,9 +9,9 @@
 extern "C" {
 #endif
 
-void isotp_complete_receive(IsoTpHandle* handle, IsoTpMessage* message);
+void isotp_complete_receive(IsoTpReceiveHandle* handle, IsoTpMessage* message);
 
-bool isotp_handle_single_frame(IsoTpHandle* handle, IsoTpMessage* message);
+bool isotp_handle_single_frame(IsoTpReceiveHandle* handle, IsoTpMessage* message);
 
 #ifdef __cplusplus
 }
diff --git a/src/isotp/send.c b/src/isotp/send.c
index b87c5602..dfba1df3 100644
--- a/src/isotp/send.c
+++ b/src/isotp/send.c
@@ -11,12 +11,11 @@ void isotp_complete_send(IsoTpShims* shims, IsoTpMessage* message,
     }
 }
 
-IsoTpHandle isotp_send_single_frame(IsoTpShims* shims, IsoTpMessage* message,
+IsoTpSendHandle isotp_send_single_frame(IsoTpShims* shims, IsoTpMessage* message,
         IsoTpMessageSentHandler callback) {
-    IsoTpHandle handle = {
+    IsoTpSendHandle handle = {
         success: false,
-        completed: true,
-        type: ISOTP_HANDLE_SENDING
+        completed: true
     };
 
     uint8_t can_data[CAN_MESSAGE_BYTE_SIZE] = {0};
@@ -42,21 +41,20 @@ IsoTpHandle isotp_send_single_frame(IsoTpShims* shims, IsoTpMessage* message,
     return handle;
 }
 
-IsoTpHandle isotp_send_multi_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");
-    IsoTpHandle handle = {
+    IsoTpSendHandle handle = {
         success: false,
-        completed: true,
-        type: ISOTP_HANDLE_SENDING
+        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)
     return handle;
 }
 
-IsoTpHandle isotp_send(IsoTpShims* shims, const uint16_t arbitration_id,
+IsoTpSendHandle isotp_send(IsoTpShims* shims, const uint16_t arbitration_id,
         const uint8_t payload[], uint16_t size,
         IsoTpMessageSentHandler callback) {
     IsoTpMessage message = {
diff --git a/tests/common.c b/tests/common.c
index 554293c9..f9898b32 100644
--- a/tests/common.c
+++ b/tests/common.c
@@ -5,7 +5,7 @@
 #include <stdarg.h>
 
 IsoTpShims SHIMS;
-IsoTpHandle HANDLE;
+IsoTpReceiveHandle RECEIVE_HANDLE;
 
 uint16_t last_can_frame_sent_arb_id;
 uint8_t last_can_payload_sent[8];
@@ -81,7 +81,7 @@ 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);
-    HANDLE = isotp_receive(&SHIMS, 0x2a, message_received);
+    RECEIVE_HANDLE = isotp_receive(&SHIMS, 0x2a, message_received);
     memset(last_message_sent_payload, 0, OUR_MAX_ISO_TP_MESSAGE_SIZE);
     memset(last_message_received_payload, 0, OUR_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 95afb949..1452ed3e 100644
--- a/tests/test_receive.c
+++ b/tests/test_receive.c
@@ -6,7 +6,7 @@
 #include <stdarg.h>
 
 extern IsoTpShims SHIMS;
-extern IsoTpHandle HANDLE;
+extern IsoTpReceiveHandle RECEIVE_HANDLE;
 
 extern uint16_t last_can_frame_sent_arb_id;
 extern uint8_t last_can_payload_sent;
@@ -28,8 +28,8 @@ extern void setup();
 START_TEST (test_receive_empty_can_message)
 {
     const uint8_t data[CAN_MESSAGE_BYTE_SIZE] = {0};
-    fail_if(HANDLE.completed);
-    IsoTpMessage message = isotp_receive_can_frame(&SHIMS, &HANDLE, 0x100, data, 0);
+    fail_if(RECEIVE_HANDLE.completed);
+    IsoTpMessage message = isotp_continue_receive(&SHIMS, &RECEIVE_HANDLE, 0x100, data, 0);
     fail_if(message.completed);
     fail_if(message_was_received);
 }
@@ -38,8 +38,8 @@ END_TEST
 START_TEST (test_receive_wrong_id)
 {
     const uint8_t data[CAN_MESSAGE_BYTE_SIZE] = {0};
-    fail_if(HANDLE.completed);
-    IsoTpMessage message = isotp_receive_can_frame(&SHIMS, &HANDLE, 0x100, data, 1);
+    fail_if(RECEIVE_HANDLE.completed);
+    IsoTpMessage message = isotp_continue_receive(&SHIMS, &RECEIVE_HANDLE, 0x100, data, 1);
     fail_if(message.completed);
     fail_if(message_was_received);
 }
@@ -49,7 +49,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};
-    IsoTpMessage message = isotp_receive_can_frame(&SHIMS, &HANDLE, 0x2a, data, 1);
+    IsoTpMessage message = isotp_continue_receive(&SHIMS, &RECEIVE_HANDLE, 0x2a, data, 1);
     fail_if(message.completed);
     fail_if(message_was_received);
 }
@@ -58,9 +58,9 @@ END_TEST
 START_TEST (test_receive_single_frame_empty_payload)
 {
     const uint8_t data[CAN_MESSAGE_BYTE_SIZE] = {0x00, 0x12, 0x34};
-    fail_if(HANDLE.completed);
-    IsoTpMessage message = isotp_receive_can_frame(&SHIMS, &HANDLE, 0x2a, data, 3);
-    fail_unless(HANDLE.completed);
+    fail_if(RECEIVE_HANDLE.completed);
+    IsoTpMessage message = isotp_continue_receive(&SHIMS, &RECEIVE_HANDLE, 0x2a, data, 3);
+    fail_unless(RECEIVE_HANDLE.completed);
     fail_unless(message.completed);
     fail_unless(message_was_received);
     ck_assert_int_eq(last_message_received_arb_id, 0x2a);
@@ -71,7 +71,7 @@ END_TEST
 START_TEST (test_receive_single_frame)
 {
     const uint8_t data[CAN_MESSAGE_BYTE_SIZE] = {0x02, 0x12, 0x34};
-    IsoTpMessage message = isotp_receive_can_frame(&SHIMS, &HANDLE, 0x2a, data, 3);
+    IsoTpMessage message = isotp_continue_receive(&SHIMS, &RECEIVE_HANDLE, 0x2a, data, 3);
     fail_unless(message.completed);
     fail_unless(message_was_received);
     ck_assert_int_eq(last_message_received_arb_id, 0x2a);
diff --git a/tests/test_send.c b/tests/test_send.c
index ca1842a6..7944e6ff 100644
--- a/tests/test_send.c
+++ b/tests/test_send.c
@@ -6,7 +6,6 @@
 #include <stdarg.h>
 
 extern IsoTpShims SHIMS;
-extern IsoTpHandle HANDLE;
 
 extern void message_sent(const IsoTpMessage* message, const bool success);
 
@@ -30,7 +29,7 @@ extern void setup();
 START_TEST (test_send_empty_payload)
 {
     uint16_t arbitration_id = 0x2a;
-    IsoTpHandle handle = isotp_send(&SHIMS, arbitration_id, NULL, 0, message_sent);
+    IsoTpSendHandle handle = 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);
@@ -49,7 +48,7 @@ START_TEST (test_send_single_frame)
 {
     const uint8_t payload[] = {0x12, 0x34};
     uint16_t arbitration_id = 0x2a;
-    IsoTpHandle handle = isotp_send(&SHIMS, arbitration_id, payload,
+    IsoTpSendHandle 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);
@@ -71,7 +70,7 @@ START_TEST (test_send_multi_frame)
     const uint8_t payload[] = {0x12, 0x34, 0x56, 0x78, 0x90, 0x01, 0x23,
             0x45, 0x67, 0x89};
     uint16_t arbitration_id = 0x2a;
-    IsoTpHandle handle = isotp_send(&SHIMS, arbitration_id, payload, sizeof(payload),
+    IsoTpSendHandle handle = isotp_send(&SHIMS, arbitration_id, payload, sizeof(payload),
             message_sent);
     fail_unless(handle.completed);
     fail_if(handle.success);