Confirm CAN messages are built correctly.

3 files changed, 40 insertions, 19 deletions

diff --git a/src/isotp/isotp.c b/src/isotp/isotp.c
index 1514625d..a1136f7b 100644
--- a/src/isotp/isotp.c
+++ b/src/isotp/isotp.c
@@ -7,11 +7,11 @@ 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;
 
-const uint8_t PCI_START_BIT = 0;
-const uint8_t PCI_WIDTH = 4;
-const uint8_t PAYLOAD_LENGTH_START_BIT = 4;
-const uint8_t PAYLOAD_LENGTH_WIDTH = 4;
-const uint8_t PAYLOAD_START_BIT = 8;
+#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,
@@ -53,14 +53,21 @@ void isotp_complete_send(IsoTpHandler* handler, const uint8_t* payload,
 
 bool isotp_send_single_frame(IsoTpHandler* handler, const uint8_t* payload,
         uint8_t size) {
-    uint64_t data;
+    uint64_t data = 0;
     setBitField(&data, PCI_SINGLE, PCI_START_BIT, PCI_WIDTH);
     setBitField(&data, size, PAYLOAD_LENGTH_START_BIT, PAYLOAD_LENGTH_WIDTH);
-    // TODO this is probably wrong
-    if(size > 0) {
-        setBitField(&data, *payload, PAYLOAD_START_BIT, size * 8);
+    // 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, payload, size);
+    handler->shims->send_can_message(handler->arbitration_id, data_array, sizeof(data_array));
     isotp_complete_send(handler, payload, size, true);
     return true;
 }
diff --git a/tests/common.c b/tests/common.c
index 512adfbc..427d90c9 100644
--- a/tests/common.c
+++ b/tests/common.c
@@ -8,7 +8,7 @@ IsoTpShims SHIMS;
 IsoTpHandler ISOTP_HANDLER;
 
 uint16_t last_can_frame_sent_arb_id;
-uint8_t last_can_payload_sent;
+uint8_t last_can_payload_sent[8];
 uint8_t last_can_payload_size;
 bool can_frame_was_sent;
 
@@ -32,6 +32,12 @@ void debug(const char* format, ...) {
 
 void mock_send_can(const uint16_t arbitration_id, const uint8_t* data,
         const uint8_t size) {
+    can_frame_was_sent = true;
+    last_can_frame_sent_arb_id = arbitration_id;
+    last_can_payload_size = size;
+    if(size > 0) {
+        memcpy(last_can_payload_sent, data, size);
+    }
 }
 
 void mock_set_timer(uint16_t time_ms, void (*callback)) {
@@ -73,13 +79,6 @@ void can_frame_sent(const uint16_t arbitration_id,
     for(int i = 0; i < size; i++) {
         debug("0x%x", payload[i]);
     }
-
-    can_frame_was_sent = true;
-    last_can_frame_sent_arb_id = arbitration_id;
-    last_can_payload_sent = size;
-    if(size > 0) {
-        memcpy(last_can_payload_sent, payload, size);
-    }
 }
 
 void setup() {
@@ -88,6 +87,7 @@ void setup() {
             can_frame_sent);
     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));
     last_message_sent_status = false;
     message_was_received = false;
     can_frame_was_sent = false;
diff --git a/tests/test_send.c b/tests/test_send.c
index c2293c21..f5037536 100644
--- a/tests/test_send.c
+++ b/tests/test_send.c
@@ -9,7 +9,7 @@ extern IsoTpShims SHIMS;
 extern IsoTpHandler ISOTP_HANDLER;
 
 extern uint16_t last_can_frame_sent_arb_id;
-extern uint8_t last_can_payload_sent;
+extern uint8_t last_can_payload_sent[8];
 extern uint8_t last_can_payload_size;
 extern bool can_frame_was_sent;
 
@@ -32,6 +32,11 @@ START_TEST (test_send_empty_payload)
     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);
+    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);
 }
 END_TEST
 
@@ -44,6 +49,13 @@ START_TEST (test_send_single_frame)
     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);
+    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);
+    ck_assert_int_eq(last_can_payload_sent[2], 0x34);
+    ck_assert_int_eq(last_can_payload_size, 3);
 }
 END_TEST
 
@@ -53,6 +65,8 @@ START_TEST (test_send_multi_frame)
             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);
 }
 END_TEST