Encapsulate arb_id, payload and size into a data type.

7 files changed, 154 insertions, 137 deletions

diff --git a/src/isotp/isotp.c b/src/isotp/isotp.c
index d552e139..7c12d430 100644
--- a/src/isotp/isotp.c
+++ b/src/isotp/isotp.c
@@ -7,87 +7,6 @@ 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;
 
-#define PCI_START_BIT 0
-#define PCI_WIDTH 4
-#define PAYLOAD_LENGTH_START_BIT PCI_START_BIT + PCI_WIDTH
-#define PAYLOAD_LENGTH_WIDTH 4
-#define PAYLOAD_START_BIT PAYLOAD_LENGTH_START_BIT + PAYLOAD_LENGTH_WIDTH
-
-void isotp_receive_can_frame(IsoTpHandler* handler,
-        const uint16_t arbitration_id, const uint64_t data,
-        const uint8_t length) {
-    if(arbitration_id != handler->arbitration_id){
-        return;
-    }
-
-    // TODO use CanMessage struct from canutil library - allocate payload buffer
-    // on stack, 8 bytes
-    // TODO  this function should receive uint64_t...
-    IsoTpProtocolControlInformation pci = (IsoTpProtocolControlInformation)
-            getBitField(data, 0, 4, false);
-
-    // TODO this is messed up! need a better API for grabbing bytes
-    uint8_t payload_length = getBitField(data, 4, 4, false);
-    uint8_t payload[payload_length];
-    uint64_t flipped_data = __builtin_bswap64(data);
-    if(payload_length > 0) {
-        memcpy(payload, &(((uint8_t*)&flipped_data)[1]), payload_length);
-    }
-
-    switch(pci) {
-        case PCI_SINGLE:
-            isotp_handle_single_frame(handler, arbitration_id, payload,
-                    payload_length);
-            break;
-        default:
-            handler->shims->log("Only single frame messages are supported");
-            break;
-    }
-}
-
-void isotp_complete_send(IsoTpHandler* handler, const uint8_t* payload,
-        uint8_t size, bool status) {
-    handler->message_sent_callback(handler->arbitration_id, payload, size,
-            status);
-}
-
-bool isotp_send_single_frame(IsoTpHandler* handler, const uint8_t* payload,
-        uint8_t size) {
-    uint64_t data = 0;
-    setBitField(&data, PCI_SINGLE, PCI_START_BIT, PCI_WIDTH);
-    setBitField(&data, size, PAYLOAD_LENGTH_START_BIT, PAYLOAD_LENGTH_WIDTH);
-    // TODO need a better bitfield API to support this - use byte array instead
-    // of uint64_t and specify desired total width
-    for(int i = 0; i < size; i++) {
-        setBitField(&data, payload[i], PAYLOAD_START_BIT + i * 8, 8);
-    }
-
-    uint8_t data_array[size + 1];
-    for(int i = 0; i < sizeof(data_array); i++) {
-        // TODO need getByte(x) function
-        data_array[i] = getBitField(data, i * 8, 8, false);
-    }
-    handler->shims->send_can_message(handler->arbitration_id, data_array, sizeof(data_array));
-    isotp_complete_send(handler, payload, size, true);
-    return true;
-}
-
-bool isotp_send_multi_frame(IsoTpHandler* handler, const uint8_t* payload,
-        uint16_t size) {
-    handler->shims->log("Only single frame messages are supported");
-    return false;
-}
-
-bool isotp_send(IsoTpHandler* handler, const uint8_t* payload,
-        uint16_t size) {
-     // we determine if it's single/multi frame and start the send
-     if(size < 8) {
-         return isotp_send_single_frame(handler, payload, size);
-     } else {
-         return isotp_send_multi_frame(handler, payload, size);
-     }
-}
-
 
 void isotp_set_timeout(IsoTpHandler* handler, uint16_t timeout_ms) {
     handler->timeout_ms = timeout_ms;
@@ -121,13 +40,12 @@ IsoTpHandler isotp_init(IsoTpShims* shims, uint16_t arbitration_id,
 }
 
 // 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", arbitration_id);
-    if(size > 0) {
+void log_isotp_message(const IsoTpMessage* message) {
+    debug("ID: 0x%02x", message->arbitration_id);
+    if(message->size > 0) {
         debug("Payload:");
-        for(int i = 0; i < size; i++) {
-            debug("0x%x", payload[i]);
+        for(int i = 0; i < message->size; i++) {
+            debug("0x%x", message->payload[i]);
         }
     }  else {
         debug("(no payload)");
diff --git a/src/isotp/isotp.h b/src/isotp/isotp.h
index 7ea0190c..a204110f 100644
--- a/src/isotp/isotp.h
+++ b/src/isotp/isotp.h
@@ -13,25 +13,21 @@ const uint16_t MAX_CAN_FRAME_SIZE;
 const uint8_t ISO_TP_DEFAULT_RESPONSE_TIMEOUT;
 const bool ISO_TP_DEFAULT_FRAME_PADDING_STATUS;
 
+typedef struct {
+    const uint16_t arbitration_id;
+    const uint8_t* payload;
+    const uint16_t size;
+} IsoTpMessage;
+
 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 uint16_t arbitration_id,
-        const uint8_t* payload, const uint16_t size, const bool success);
-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 void (*IsoTpMessageReceivedHandler)(const IsoTpMessage* message);
+typedef void (*IsoTpMessageSentHandler)(const IsoTpMessage* message,
+        const bool success);
+typedef void (*IsoTpCanFrameSentHandler)(const IsoTpMessage* message);
 
 typedef struct {
     LogShim log;
@@ -88,19 +84,9 @@ IsoTpHandler isotp_init(IsoTpShims* shims,
  */
 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
-bool isotp_send(IsoTpHandler* handler, const uint8_t* payload,
-        uint16_t payload_size);
-
-void isotp_receive_can_frame(IsoTpHandler* handler,
-        const uint16_t arbitration_id, const uint64_t data,
-        const uint8_t length);
-
 void isotp_destroy(IsoTpHandler* handler);
 
-void log_isotp_message(const uint16_t arbitration_id, const uint8_t* payload,
-        const uint16_t size);
+void log_isotp_message(const IsoTpMessage* message);
 
 
 #ifdef __cplusplus
diff --git a/src/isotp/receive.c b/src/isotp/receive.c
index 9add9d5f..b4978097 100644
--- a/src/isotp/receive.c
+++ b/src/isotp/receive.c
@@ -1,7 +1,47 @@
 #include <isotp/receive.h>
 
-void isotp_handle_single_frame(IsoTpHandler* handler,
-        const uint16_t arbitration_id, const uint8_t* data,
+void isotp_handle_single_frame(IsoTpHandler* handler, IsoTpMessage* message) {
+    isotp_complete_receive(handler, message);
+}
+
+void isotp_complete_receive(IsoTpHandler* handler, IsoTpMessage* message) {
+    handler->message_received_callback(message);
+}
+
+void isotp_receive_can_frame(IsoTpHandler* handler,
+        const uint16_t arbitration_id, const uint64_t data,
         const uint8_t length) {
-    handler->message_received_callback(arbitration_id, data, length);
+    if(arbitration_id != handler->arbitration_id){
+        return;
+    }
+
+    // TODO use CanMessage struct from canutil library - allocate payload buffer
+    // on stack, 8 bytes
+    // TODO  this function should receive uint64_t...
+    IsoTpProtocolControlInformation pci = (IsoTpProtocolControlInformation)
+            getBitField(data, 0, 4, false);
+
+    // TODO this is messed up! need a better API for grabbing bytes
+    uint8_t payload_length = getBitField(data, 4, 4, false);
+    uint8_t payload[payload_length];
+    uint64_t flipped_data = __builtin_bswap64(data);
+    if(payload_length > 0) {
+        memcpy(payload, &(((uint8_t*)&flipped_data)[1]), payload_length);
+    }
+
+    IsoTpMessage message = {
+        arbitration_id: arbitration_id,
+        payload: payload,
+        size: payload_length
+    };
+
+    switch(pci) {
+        case PCI_SINGLE:
+            isotp_handle_single_frame(handler, &message);
+            break;
+        default:
+            handler->shims->log("Only single frame messages are supported");
+            break;
+    }
 }
+
diff --git a/src/isotp/receive.h b/src/isotp/receive.h
index f7d6c61f..0ff9e8f1 100644
--- a/src/isotp/receive.h
+++ b/src/isotp/receive.h
@@ -9,8 +9,12 @@
 extern "C" {
 #endif
 
-void isotp_handle_single_frame(IsoTpHandler* handler, const uint16_t arbitration_id,
-        const uint8_t* data, const uint8_t length);
+void isotp_handle_single_frame(IsoTpHandler* handler, IsoTpMessage* message);
+
+void isotp_complete_receive(IsoTpHandler* handler, IsoTpMessage* message);
+
+void isotp_receive_can_frame(IsoTpHandler* handler,
+        const uint16_t arbitration_id, const uint64_t data, const uint8_t size);
 
 #ifdef __cplusplus
 }
diff --git a/src/isotp/send.c b/src/isotp/send.c
new file mode 100644
index 00000000..b72d1b91
--- /dev/null
+++ b/src/isotp/send.c
@@ -0,0 +1,54 @@
+#include <isotp/send.h>
+
+#define PCI_START_BIT 0
+#define PCI_WIDTH 4
+#define PAYLOAD_LENGTH_START_BIT PCI_START_BIT + PCI_WIDTH
+#define PAYLOAD_LENGTH_WIDTH 4
+#define PAYLOAD_START_BIT PAYLOAD_LENGTH_START_BIT + PAYLOAD_LENGTH_WIDTH
+
+void isotp_complete_send(IsoTpHandler* handler, IsoTpMessage* message,
+        bool status) {
+    handler->message_sent_callback(message, status);
+}
+
+bool isotp_send_single_frame(IsoTpHandler* handler, IsoTpMessage* message) {
+    uint64_t data = 0;
+    setBitField(&data, PCI_SINGLE, PCI_START_BIT, PCI_WIDTH);
+    setBitField(&data, message->size, PAYLOAD_LENGTH_START_BIT, PAYLOAD_LENGTH_WIDTH);
+    // TODO need a better bitfield API to support this - use byte array instead
+    // of uint64_t and specify desired total width
+    for(int i = 0; i < message->size; i++) {
+        setBitField(&data, message->payload[i], PAYLOAD_START_BIT + i * 8, 8);
+    }
+
+    uint8_t data_array[message->size + 1];
+    for(int i = 0; i < sizeof(data_array); i++) {
+        // TODO need getByte(x) function
+        data_array[i] = getBitField(data, i * 8, 8, false);
+    }
+    handler->shims->send_can_message(message->arbitration_id, data_array, sizeof(data_array));
+    isotp_complete_send(handler, message, true);
+    return true;
+}
+
+bool isotp_send_multi_frame(IsoTpHandler* handler, IsoTpMessage* message) {
+    // TODO make sure to copy payload into a local buffer
+    handler->shims->log("Only single frame messages are supported");
+    return false;
+}
+
+bool isotp_send(IsoTpHandler* handler, const uint8_t* payload,
+        uint16_t size) {
+    // we determine if it's single/multi frame and start the send
+    IsoTpMessage message = {
+        arbitration_id: handler->arbitration_id,
+        payload: payload,
+        size: size
+    };
+
+    if(size < 8) {
+        return isotp_send_single_frame(handler, &message);
+    } else {
+        return isotp_send_multi_frame(handler, &message);
+    }
+}
diff --git a/src/isotp/send.h b/src/isotp/send.h
new file mode 100644
index 00000000..17a7dad0
--- /dev/null
+++ b/src/isotp/send.h
@@ -0,0 +1,19 @@
+#ifndef __ISOTP_SEND_H__
+#define __ISOTP_SEND_H__
+
+#include <isotp/isotp.h>
+#include <stdint.h>
+#include <stdbool.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+bool isotp_send(IsoTpHandler* handler, const uint8_t* payload,
+        uint16_t payload_size);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif // __ISOTP_SEND_H__
diff --git a/tests/common.c b/tests/common.c
index 427d90c9..4c6a40ff 100644
--- a/tests/common.c
+++ b/tests/common.c
@@ -43,42 +43,38 @@ void mock_send_can(const uint16_t arbitration_id, const uint8_t* data,
 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) {
+void message_received(const IsoTpMessage* message) {
     debug("Received ISO-TP message:");
     message_was_received = true;
-    log_isotp_message(arbitration_id, payload, size);
-    last_message_received_arb_id = arbitration_id;
-    last_message_received_payload_size = size;
-    if(size > 0) {
-        memcpy(last_message_received_payload, payload, size);
+    log_isotp_message(message);
+    last_message_received_arb_id = message->arbitration_id;
+    last_message_received_payload_size = message->size;
+    if(message->size > 0) {
+        memcpy(last_message_received_payload, message->payload, message->size);
     }
 }
 
-void message_sent(const uint16_t arbitration_id, const uint8_t* payload,
-        const uint16_t size, const bool success) {
+void message_sent(const IsoTpMessage* message, const bool success) {
     if(success) {
         debug("Sent ISO-TP message:");
     } else {
         debug("Unable to send ISO-TP message:");
     }
-    log_isotp_message(arbitration_id, payload, size);
+    log_isotp_message(message);
 
-    last_message_sent_arb_id = arbitration_id;
-    last_message_sent_payload_size = size;
+    last_message_sent_arb_id = message->arbitration_id;
+    last_message_sent_payload_size = message->size;
     last_message_sent_status = success;
-    if(size > 0) {
-        memcpy(last_message_sent_payload, payload, size);
+    if(message->size > 0) {
+        memcpy(last_message_sent_payload, message->payload, message->size);
     }
 }
 
-void can_frame_sent(const uint16_t arbitration_id,
-        const uint8_t* payload, const uint8_t 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]);
-    }
+    // TODO add something like this to canutil
+    /* log_can_message(arbitration_id, payload, size); */
 }
 
 void setup() {