Draft reworking of API.

2 files changed, 77 insertions, 29 deletions

diff --git a/src/obd2/obd2.c b/src/obd2/obd2.c
index 3d501d6a..33e6da15 100644
--- a/src/obd2/obd2.c
+++ b/src/obd2/obd2.c
@@ -1,14 +1,15 @@
 #include <obd2/obd2.h>
 
+#define MAX_DIAGNOSTIC_PAYLOAD_SIZE 6
+#define MODE_BYTE_INDEX 0
+#define PID_BYTE_INDEX 1
+
 DiagnosticShims diagnostic_init_shims(LogShim log,
         SendCanMessageShim send_can_message,
         SetTimerShim set_timer) {
     DiagnosticShims shims = {
-        isotp_shims: {
-            log: log,
-            send_can_message: send_can_message,
-            set_timer: set_timer
-        },
+        send_can_message: send_can_message,
+        set_timer: set_timer,
         log: log
     };
     return shims;
@@ -16,34 +17,60 @@ DiagnosticShims diagnostic_init_shims(LogShim log,
 
 DiagnosticRequestHandle diagnostic_request(DiagnosticShims* shims,
         DiagnosticRequest* request, DiagnosticResponseReceived callback) {
-    // TODO hmm, where is message_received coming from? we would need 2 layers
-    // of callbacks. if we do it in the obd2 library, we have to have some
-    // context passed to that message_received handler so we can then retreive
-    // the obd2 callback. there was an option question of if we should pass a
-    // context with that callback, and maybe this answers it.
-    //
-    // alternatively, what if don't hide isotp and allow that to also be
-    // injected. the user has the iso_tp message_received callback, and in that
-    // they call a message_received handler from obd2.
-    //
-    // in fact that makes more sense - all the diagnostic_can_frame_received
-    // function is going to be able to do is call the equivalent function in the
-    // isotp library. it may or may not have a complete ISO-TP message. huh.
     DiagnosticRequestHandle handle = {
-        // TODO why are teh shims stored as a reference in the isotp handler?
-        // it's just 3 pointers
-        isotp_handler: isotp_init(&shims->isotp_shims, request->arbitration_id,
-               NULL, // TODO need a callback here!
-               NULL, NULL),
-        type: 0 //DIAGNOSTIC_.... // TODO how would we know the type?
-            //does it matter? we were going to have a different callback
+        type: DIAGNOSTIC_REQUEST_TYPE_PID,
+        callback: callback,
+        status: true
     };
+    uint8_t payload[MAX_DIAGNOSTIC_PAYLOAD_SIZE];
+    payload[MODE_BYTE_INDEX] = request->mode;
+    if(request->pid_length > 0) {
+        copy_bytes_right_aligned(request->pid, sizeof(request->pid),
+                PID_BYTE_INDEX, request->pid_length, payload, sizeof(payload));
+    }
+    if(request->payload_length > 0) {
+        memcpy(payload[PID_BYTE_INDEX + request->pid_length],
+                request->payload, request->payload_length);
+    }
+
+    IsoTpShims isotp_shims = isotp_init_shims(shims->log,
+            shims->send_can_message,
+            shims->set_timer);
+    handle.status = isotp_send(&isotp_shims, request->arbitration_id,
+            payload, 1 + request->payload_length + request->pid_length,
+            diagnostic_receive_isotp_message);
+
+    // TODO need to set up an isotp receive handler. in isotp, rx and tx are
+    // kind of intermingled at this point. really, there's not explicit link
+    // between send and receveice...well except for flow control. hm, damn.
+    // so there's 2 things:
+    //
+    // isotp_send needs to return a handle. if it was a single frame, we
+    // probably sent it right away so the status true and the callback was hit.
+    // the handle needs another flag to say if it was completed or not, so you
+    // know you can destroy it. you will continue to throw can frames at that
+    // handler until it returns completed (either with a  flag, or maybe
+    // receive_can_frame returns true if it's complete)
+    //
+    // the second thing is that we need to be able to arbitrarly set up to
+    // receive an iso-tp message on a particular arb id. again, you keep
+    // throwing can frames at it until it returns a handle with the status
+    // completed and calls your callback
+    //
+    // so the diagnostic request needs 2 isotp handles and they should both be
+    // hidden from the user
+    //
+    // when a can frame is received and passes to the diagnostic handle
+    // if we haven't successfuly sent the entire message yet, give it to the
+    // isottp send handle
+    // if we have sent it, give it to the isotp rx handle
+    // if we've received properly, mark this request as completed
+    return handle;
 }
 
 DiagnosticRequestHandle diagnostic_request_pid(DiagnosticShims* shims,
         DiagnosticPidRequestType pid_request_type, uint16_t pid,
         DiagnosticResponseReceived callback) {
-    // decide mode 0x1 / 0x22 based on pid type
     DiagnosticRequest request = {
         mode: pid_request_type == DIAGNOSTIC_STANDARD_PID ? 0x1 : 0x22,
         pid: pid
@@ -52,12 +79,21 @@ DiagnosticRequestHandle diagnostic_request_pid(DiagnosticShims* shims,
     return diagnostic_request(shims, &request, callback);
 }
 
-void diagnostic_receive_can_frame(DiagnosticRequestHandle* handler,
+void diagnostic_receive_isotp_message(const IsoTpMessage* message) {
+    // TODO
+}
+
+void diagnostic_receive_can_frame(DiagnosticRequestHandle* handle,
         const uint16_t arbitration_id, const uint8_t data[],
         const uint8_t size) {
-    isotp_receive_can_frame(handler->isotp_handler, arbitration_id, data, size);
+    isotp_receive_can_frame(handle->isotp_handler, arbitration_id, data, size);
 }
 
+// TODO argh, we're now saying that user code will rx CAN messages, but who does
+// it hand them to? isotp handlers are encapsulated in diagnostic handles
+
+
+
 // TODO everything below here is for future work...not critical for now.
 
 DiagnosticRequestHandle diagnostic_request_malfunction_indicator_status(
diff --git a/src/obd2/obd2.h b/src/obd2/obd2.h
index e4ffc7ab..8d7d6642 100644
--- a/src/obd2/obd2.h
+++ b/src/obd2/obd2.h
@@ -16,10 +16,15 @@ typedef struct {
     uint16_t arbitration_id;
     uint8_t mode;
     uint16_t pid;
+    uint8_t pid_length;
     uint8_t payload[MAX_OBD2_PAYLOAD_LENGTH];
     uint8_t payload_length;
 } DiagnosticRequest;
 
+// TODO I don't like this, it's hard coding isotp library stuff here
+typedef bool (*SendIsoTpMessageShim)(IsoTpHandler* handler,
+        const uint8_t* payload, uint16_t payload_size);
+
 // Thanks to
 // http://www.canbushack.com/blog/index.php?title=scanning-for-diagnostic-data&more=1&c=1&tb=1&pb=1
 // for the list of NRCs
@@ -116,6 +121,7 @@ typedef struct {
     // compare an incoming CAN message to see if it matches the service / PID!
     // TODO i'm not sure this type/callback in here is too useful - see the
     // comments in obd2.c:diagnostic_request
+
     DiagnosticRequestType type;
     DiagnosticResponseReceived callback;
     DiagnosticMilStatusReceived mil_status_callback;
@@ -129,7 +135,8 @@ typedef enum {
 } DiagnosticPidRequestType;
 
 typedef struct {
-    IsoTpShims isotp_shims;
+    SetTimerShim set_timer;
+    SendCanMessageShim send_can_message;
     LogShim log;
 } DiagnosticShims;
 
@@ -161,6 +168,11 @@ bool diagnostic_clear_dtc(DiagnosticShims* shims);
 DiagnosticRequestHandle diagnostic_enumerate_pids(DiagnosticShims* shims,
         DiagnosticRequest* request, DiagnosticPidEnumerationReceived callback);
 
+// TODO
+// void diagnostic_receive_isotp_message(DiagnosticRequestHandle* handle,
+        // const IsoTpMessage* message);
+void diagnostic_receive_isotp_message(const IsoTpMessage* message);
+
 void diagnostic_receive_can_frame(DiagnosticRequestHandle* handle,
         const uint16_t arbitration_id, const uint8_t data[],
         const uint8_t size);