DiagnosticRequestHandle handle = {
type: DIAGNOSTIC_REQUEST_TYPE_PID,
callback: callback,
- status: true
+ success: false,
+ completed: false
};
uint8_t payload[MAX_DIAGNOSTIC_PAYLOAD_SIZE];
payload[MODE_BYTE_INDEX] = request->mode;
request->payload, request->payload_length);
}
- IsoTpShims isotp_shims = isotp_init_shims(shims->log,
+ handle.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
- //
+ handle.isotp_send_handle = isotp_send(&handle.isotp_shims,
+ request->arbitration_id, payload,
+ 1 + request->payload_length + request->pid_length,
+ // TODO is this ok to pass null here?
+ NULL);
+
+ handle.isotp_receive_handle = isotp_receive(&handle.isotp_shims,
+ // TODO need to either always add 0x8 or let the user specify
+ request->arbitration_id + 0x8,
+ // TODO this callback is mostly useful for debugging stuff as it
+ // doesn't have the internal state we need to complete the
+ // diagnositc request - can we pass NULL or will that 'splode?
+ NULL);
+
// 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
+ // how do you get the result? we have it set up for callbacks but that's
+ // getting to be kind of awkward
+ //
+ // say it were to call a callback...what state would it need to pass?
+ //
+ // the iso-tp message received callback needs to pass the handle and the
+ // received message
+ //
+ // so in the obd2 library, we get a callback with an isotp message. how do
+ // we know what diag request i went with, and which diag callback to use? we
+ // could store context with the isotp handle. the problem is that context is
+ // self referential and on the stack, so we really can't get a pointer to
+ // it.
+ //
+ // the diagnostic response received callback needs to pass the diagnostic
+ // handle and the diag response
+ //
+ // let's say we simplify things and drop the callbacks.
+ //
+ // isotp_receive_can_frame returns an isotp handle with a complete message
+ // in it if one was received
+ //
+ // diagnostic_receive_can_frame returns a diaghandle if one was received
+ //
+ // so in the user code you would throw the can frame at each of your active
+ // diag handles and see if any return a completed message.
+ //
+ // is there any advantage to a callback approach? callbacks are useful when
+ // you have something that will block, bt we don't have anything that will
+ // block. it's async but we return immediately from each partial parsing
+ // attempt.
+ //
+ // argh, but will we need the callbacks when we add timers for isotp multi
+ // frame?
+ //
+ // what are the timers for exactly?
+ //
+ // when sending multi frame, send 1 frame, wait for a response
+ // if it says send all, send all right away
+ // if it says flow control, set the time for the next send
+ // instead of creating a timer with an async callback, add a process_handle
+ // function that's called repeatedly in the main loop - if it's time to
+ // send, we do it. so there's a process_handle_send and receive_can_frame
+ // that are just called continuously from the main loop. it's a waste of a
+ // few cpu cycles but it may be more natural than callbacks.
+ //
+ // what woudl a timer callback look like...it would need to pass the handle
+ // and that's all. seems like a context void* would be able to capture all
+ // of the information but arg, memory allocation. look at how it's done in
+ // the other library again
+ //
return handle;
}
return diagnostic_request(shims, &request, callback);
}
-void diagnostic_receive_isotp_message(const IsoTpMessage* message) {
- // TODO
-}
-
-void diagnostic_receive_can_frame(DiagnosticRequestHandle* handle,
+DiagnosticResponse diagnostic_receive_can_frame(DiagnosticShims* shims,
+ DiagnosticRequestHandle* handle,
const uint16_t arbitration_id, const uint8_t data[],
const uint8_t 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
+ if(!handle->isotp_send_handle.completed) {
+ } else if(!handle->isotp_receive_handle.completed) {
+ } else {
+ shims->log("Handle is already completed");
+ }
+ // TODO determine which isotp handler to pass it to based on our state
+ IsoTpMessage message = isotp_receive_can_frame(&handle->isotp_shims,
+ &handle->isotp_send_handle, arbitration_id, data, size);
+ DiagnosticResponse response = {
+ success: false,
+ completed: false
+ };
+ if(message.completed) {
+ }
+}
// TODO everything below here is for future work...not critical for now.
} DiagnosticRequest;
// TODO I don't like this, it's hard coding isotp library stuff here
-typedef bool (*SendIsoTpMessageShim)(IsoTpHandler* handler,
+typedef bool (*SendIsoTpMessageShim)(IsoTpHandle* handle,
const uint8_t* payload, uint16_t payload_size);
// Thanks to
uint16_t arbitration_id;
uint8_t mode;
bool success;
+ bool completed;
// if mode is one with a PID, read the correct numbers of PID bytes (1 or 2)
// into this field, then store the remainder of the payload in the payload
// field
} DiagnosticRequestType;
typedef struct {
- IsoTpHandler isotp_handler;
+ bool success;
+ bool completed;
+ IsoTpShims isotp_shims;
+ IsoTpHandle isotp_send_handle;
+ IsoTpHandle isotp_receive_handle;
// TODO the Handle may need to keep the original request, otherwise we can't
// 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
DiagnosticResponseReceived callback;
DiagnosticMilStatusReceived mil_status_callback;
DiagnosticVinReceived vin_callback;
- bool status;
} DiagnosticRequestHandle;
typedef enum {
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,
+DiagnosticResponse diagnostic_receive_can_frame(DiagnosticShims* shims,
+ DiagnosticRequestHandle* handle,
const uint16_t arbitration_id, const uint8_t data[],
const uint8_t size);
fail_if(last_response_was_received);
const uint8_t can_data[] = {0x2, request.mode + 0x40, 0x23};
- diagnostic_receive_can_frame(&handle, request.arbitration_id, can_data,
- sizeof(can_data));
+ diagnostic_receive_can_frame(&SHIMS, &handle, request.arbitration_id,
+ can_data, sizeof(can_data));
fail_if(last_response_was_received);
}
END_TEST
DiagnosticRequestHandle handle = diagnostic_request(&SHIMS, &request,
response_received_handler);
+ fail_if(handle.completed);
+
fail_if(last_response_was_received);
const uint8_t can_data[] = {0x2, request.mode + 0x40, 0x23};
- diagnostic_receive_can_frame(&handle, request.arbitration_id + 0x8,
- can_data, sizeof(can_data));
+ DiagnosticResponse response = diagnostic_receive_can_frame(&SHIMS, &handle,
+ request.arbitration_id + 0x8, can_data, sizeof(can_data));
+ fail_unless(response.success);
+ fail_unless(response.completed);
+ fail_unless(handle.completed);
fail_unless(last_response_was_received);
ck_assert(last_response_received.success);
ck_assert_int_eq(last_response_received.arbitration_id,
fail_if(last_response_was_received);
const uint8_t can_data[] = {0x3, 0x1 + 0x40, 0x2, 0x45};
// TODO need a constant for the 7df broadcast functional request
- diagnostic_receive_can_frame(&handle, 0x7df + 0x8,
+ diagnostic_receive_can_frame(&SHIMS, &handle, 0x7df + 0x8,
can_data, sizeof(can_data));
fail_unless(last_response_was_received);
ck_assert(last_response_received.success);
fail_if(last_response_was_received);
const uint8_t can_data[] = {0x4, 0x1 + 0x40, 0x0, 0x2, 0x45};
// TODO need a constant for the 7df broadcast functional request
- diagnostic_receive_can_frame(&handle, 0x7df + 0x8, can_data,
+ diagnostic_receive_can_frame(&SHIMS, &handle, 0x7df + 0x8, can_data,
sizeof(can_data));
fail_unless(last_response_was_received);
ck_assert(last_response_received.success);
Code Review / apps / low-level-can-service.git / commitdiff