Draft reworking of API.

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 <isotp/receive.h>
 
-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 <stdarg.h>
 
 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 <stdarg.h>
 
 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 <stdarg.h>
 
 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