Skip to content

Commit

Permalink
Multi frame (openxc#6)
Browse files Browse the repository at this point in the history 
* increase payload size just for diag responses to support multi-frame responses. limited to 255 bytes for now.

* not all diag requests with modes greater than 0xa have a 2 byte pid. need to check was pid_length should be based on the size of the pid.

* remove line that does nothing.

* add multi_frame field to DiagnosticResponse and update based on IsoTpMessage. Need this upstream for timeout calls specific to multi frame.

* update isotp-c submodule.

* update autoset_pid tests. check that pid_length is dynamically set based on pid value. adjust other tests to use 2-byte pid for enhanced diagnostic mode requests.

* add test for multi-frame response.

* update changelog.
  • Loading branch information
Zac Nelson authored Sep 14, 2016
1 parent b2033dd commit 6e6ffb3
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 21 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.mkd
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# OBD-II Support Library in C

## v0.2

* Add support for multi-frame diagnostic responses.

## v0.1

* Initial release
2 changes: 1 addition & 1 deletion deps/isotp-c
Submodule isotp-c updated 6 files
+0 −3 .travis.yml
+5 −0 CHANGELOG.mkd
+2 −1 src/isotp/isotp.c
+12 −4 src/isotp/isotp_types.h
+92 −9 src/isotp/receive.c
+64 −0 tests/test_receive.c
7 changes: 5 additions & 2 deletions src/uds/uds.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ static void setup_receive_handle(DiagnosticRequestHandle* handle) {
static uint16_t autoset_pid_length(uint8_t mode, uint16_t pid,
uint8_t pid_length) {
if(pid_length == 0) {
if(pid > 0xffff || (mode != 0x3e && mode > 0xa)) {
if(mode <= 0xa || mode == 0x3e ) {
pid_length = 1;
} else if(pid > 0xffff || ((pid & 0xFF00) > 0x0)) {
pid_length = 2;
} else {
pid_length = 1;
Expand All @@ -69,7 +71,6 @@ static void send_diagnostic_request(DiagnosticShims* shims,
if(handle->request.has_pid) {
handle->request.pid_length = autoset_pid_length(handle->request.mode,
handle->request.pid, handle->request.pid_length);
handle->request.pid_length = handle->request.pid_length;
set_bitfield(handle->request.pid, PID_BYTE_INDEX * CHAR_BIT,
handle->request.pid_length * CHAR_BIT, payload,
sizeof(payload));
Expand Down Expand Up @@ -233,6 +234,7 @@ DiagnosticResponse diagnostic_receive_can_frame(DiagnosticShims* shims,

DiagnosticResponse response = {
arbitration_id: arbitration_id,
multi_frame: false,
success: false,
completed: false
};
Expand All @@ -246,6 +248,7 @@ DiagnosticResponse diagnostic_receive_can_frame(DiagnosticShims* shims,
IsoTpMessage message = isotp_continue_receive(&handle->isotp_shims,
&handle->isotp_receive_handles[i], arbitration_id, data,
size);
response.multi_frame = message.multi_frame;

if(message.completed) {
if(message.size > 0) {
Expand Down
14 changes: 9 additions & 5 deletions src/uds/uds_types.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@
extern "C" {
#endif

// TODO This isn't true for multi frame messages - we may need to dynamically
// allocate this in the future
#define MAX_UDS_PAYLOAD_LENGTH 7
// TODO This still doesn't have enough space for the largest possible
// multiframe response. May need to dynamically allocate in the future.
#define MAX_UDS_RESPONSE_PAYLOAD_LENGTH 255
#define MAX_UDS_REQUEST_PAYLOAD_LENGTH 7
#define MAX_RESPONDING_ECU_COUNT 8
#define VIN_LENGTH 17

Expand Down Expand Up @@ -54,7 +55,7 @@ typedef struct {
bool has_pid;
uint16_t pid;
uint8_t pid_length;
uint8_t payload[MAX_UDS_PAYLOAD_LENGTH];
uint8_t payload[MAX_UDS_REQUEST_PAYLOAD_LENGTH];
uint8_t payload_length;
bool no_frame_padding;
DiagnosticRequestType type;
Expand Down Expand Up @@ -92,6 +93,8 @@ typedef enum {
* field isn't valid if 'completed' isn't true. If this is 'false', check
* the negative_response_code field for the reason.
* arbitration_id - The arbitration ID the response was received on.
* multi_frame - True if this response (whether completed or not) required
* multi-frame CAN support. Can be used for updating time-out functions.
* mode - The OBD-II mode for the original request.
* has_pid - If this is a response to a PID request, this will be true and the
* 'pid' field will be valid.
Expand All @@ -106,12 +109,13 @@ typedef enum {
typedef struct {
bool completed;
bool success;
bool multi_frame;
uint32_t arbitration_id;
uint8_t mode;
bool has_pid;
uint16_t pid;
DiagnosticNegativeResponseCode negative_response_code;
uint8_t payload[MAX_UDS_PAYLOAD_LENGTH];
uint8_t payload[MAX_UDS_RESPONSE_PAYLOAD_LENGTH];
uint8_t payload_length;
} DiagnosticResponse;

Expand Down
98 changes: 85 additions & 13 deletions tests/test_core.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,14 +242,26 @@ START_TEST (test_autoset_pid_length)
arbitration_id: 0x100,
mode: 0x22,
has_pid: true,
pid: 2,
pid: 0x1234,
no_frame_padding: true
};
diagnostic_request(&SHIMS, &request, response_received_handler);

ck_assert_int_eq(last_can_frame_sent_arb_id, request.arbitration_id);
ck_assert_int_eq(last_can_payload_sent[1], request.mode);
ck_assert_int_eq(last_can_payload_sent[2], (request.pid & 0xFF00) >> 8);
ck_assert_int_eq(last_can_payload_sent[3], request.pid & 0xFF);
ck_assert_int_eq(last_can_payload_size, 4);

request.arbitration_id = 0x101;
request.pid = 0x12;

diagnostic_request(&SHIMS, &request, response_received_handler);

ck_assert_int_eq(last_can_frame_sent_arb_id, request.arbitration_id);
ck_assert_int_eq(last_can_payload_sent[1], request.mode);
ck_assert_int_eq(last_can_payload_sent[2], request.pid);
ck_assert_int_eq(last_can_payload_size, 3);
}
END_TEST

Expand Down Expand Up @@ -279,10 +291,10 @@ START_TEST (test_request_pid_enhanced)
{
uint16_t arb_id = 0x100;
DiagnosticRequestHandle handle = diagnostic_request_pid(&SHIMS,
DIAGNOSTIC_ENHANCED_PID, arb_id, 0x2, response_received_handler);
DIAGNOSTIC_ENHANCED_PID, arb_id, 0x1234, response_received_handler);

fail_if(last_response_was_received);
const uint8_t can_data[] = {0x4, 0x22 + 0x40, 0x0, 0x2, 0x45};
const uint8_t can_data[] = {0x4, 0x22 + 0x40, 0x12, 0x34, 0x45};
diagnostic_receive_can_frame(&SHIMS, &handle, arb_id + 0x8, can_data,
sizeof(can_data));
fail_unless(last_response_was_received);
Expand All @@ -291,7 +303,7 @@ START_TEST (test_request_pid_enhanced)
arb_id + 0x8);
ck_assert_int_eq(last_response_received.mode, 0x22);
fail_unless(last_response_received.has_pid);
ck_assert_int_eq(last_response_received.pid, 0x2);
ck_assert_int_eq(last_response_received.pid, 0x1234);
ck_assert_int_eq(last_response_received.payload_length, 1);
ck_assert_int_eq(last_response_received.payload[0], can_data[4]);
}
Expand All @@ -301,10 +313,10 @@ START_TEST (test_wrong_mode_response)
{
uint16_t arb_id = 0x100;
DiagnosticRequestHandle handle = diagnostic_request_pid(&SHIMS,
DIAGNOSTIC_ENHANCED_PID, arb_id, 0x2, response_received_handler);
DIAGNOSTIC_ENHANCED_PID, arb_id, 0x1234, response_received_handler);

fail_if(last_response_was_received);
const uint8_t can_data[] = {0x4, 0x1 + 0x40, 0x0, 0x2, 0x45};
const uint8_t can_data[] = {0x4, 0x1 + 0x40, 0x12, 0x34, 0x45};
diagnostic_receive_can_frame(&SHIMS, &handle, arb_id + 0x8, can_data,
sizeof(can_data));
fail_if(last_response_was_received);
Expand All @@ -316,7 +328,7 @@ START_TEST (test_missing_pid)
{
uint16_t arb_id = 0x100;
DiagnosticRequestHandle handle = diagnostic_request_pid(&SHIMS,
DIAGNOSTIC_ENHANCED_PID, arb_id, 0x2, response_received_handler);
DIAGNOSTIC_ENHANCED_PID, arb_id, 0x1234, response_received_handler);

fail_if(last_response_was_received);
const uint8_t can_data[] = {0x1, 0x22 + 0x40};
Expand All @@ -331,10 +343,10 @@ START_TEST (test_wrong_pid_response)
{
uint16_t arb_id = 0x100;
DiagnosticRequestHandle handle = diagnostic_request_pid(&SHIMS,
DIAGNOSTIC_ENHANCED_PID, arb_id, 0x2, response_received_handler);
DIAGNOSTIC_ENHANCED_PID, arb_id, 0x1234, response_received_handler);

fail_if(last_response_was_received);
const uint8_t can_data[] = {0x4, 0x22 + 0x40, 0x0, 0x3, 0x45};
const uint8_t can_data[] = {0x4, 0x22 + 0x40, 0x12, 0x33, 0x45};
diagnostic_receive_can_frame(&SHIMS, &handle, arb_id + 0x8, can_data,
sizeof(can_data));
fail_if(last_response_was_received);
Expand All @@ -346,23 +358,23 @@ START_TEST (test_wrong_pid_then_right_completes)
{
uint16_t arb_id = 0x100;
DiagnosticRequestHandle handle = diagnostic_request_pid(&SHIMS,
DIAGNOSTIC_ENHANCED_PID, arb_id, 0x2, response_received_handler);
DIAGNOSTIC_ENHANCED_PID, arb_id, 0x1234, response_received_handler);

fail_if(last_response_was_received);
uint8_t can_data[] = {0x4, 0x22 + 0x40, 0x0, 0x3, 0x45};
uint8_t can_data[] = {0x4, 0x22 + 0x40, 0x12, 0x33, 0x45};
diagnostic_receive_can_frame(&SHIMS, &handle, arb_id + 0x8, can_data,
sizeof(can_data));
fail_if(last_response_was_received);
fail_if(handle.completed);

can_data[3] = 0x2;
can_data[3] = 0x34;
diagnostic_receive_can_frame(&SHIMS, &handle, arb_id + 0x8, can_data,
sizeof(can_data));
fail_unless(last_response_was_received);
fail_unless(handle.completed);
fail_unless(handle.success);
fail_unless(last_response_received.success);
ck_assert_int_eq(last_response_received.pid, 0x2);
ck_assert_int_eq(last_response_received.pid, 0x1234);
}
END_TEST

Expand Down Expand Up @@ -407,6 +419,65 @@ START_TEST (test_payload_to_integer)
}
END_TEST

START_TEST (test_response_multi_frame)
{
DiagnosticRequest request = {
arbitration_id: 0x100,
mode: OBD2_MODE_VEHICLE_INFORMATION,
has_pid: true,
pid: 0x2
};
DiagnosticRequestHandle handle = diagnostic_request(&SHIMS, &request,
response_received_handler);

const uint8_t can_data[] = {0x10, 0x14, 0x9 + 0x40, 0x2, 0x1, 0x31, 0x46, 0x4d};
DiagnosticResponse response = diagnostic_receive_can_frame(&SHIMS, &handle,
request.arbitration_id + 0x8, can_data, sizeof(can_data));

fail_unless(can_frame_was_sent);
fail_unless(!response.success);
fail_unless(!response.completed);
fail_unless(response.multi_frame);
ck_assert_int_eq(last_can_frame_sent_arb_id, request.arbitration_id);
ck_assert_int_eq(last_can_payload_sent[0], 0x30);

const uint8_t can_data_1[] = {0x21, 0x43, 0x55, 0x39, 0x4a, 0x39, 0x34, 0x48};
response = diagnostic_receive_can_frame(&SHIMS, &handle,
request.arbitration_id + 0x8, can_data_1, sizeof(can_data_1));
fail_unless(!response.success);
fail_unless(!response.completed);
fail_unless(response.multi_frame);

const uint8_t can_data_2[] = {0x22, 0x55, 0x41, 0x30, 0x34, 0x35, 0x32, 0x34};
response = diagnostic_receive_can_frame(&SHIMS, &handle,
request.arbitration_id + 0x8, can_data_2, sizeof(can_data_2));
fail_unless(response.success);
fail_unless(response.completed);
fail_unless(response.multi_frame);
ck_assert_int_eq(response.mode, OBD2_MODE_VEHICLE_INFORMATION);
ck_assert_int_eq(response.pid, 0x2);
ck_assert_int_eq(response.payload_length, 18);
ck_assert_int_eq(response.payload[0], 0x01);
ck_assert_int_eq(response.payload[1], 0x31);
ck_assert_int_eq(response.payload[2], 0x46);
ck_assert_int_eq(response.payload[3], 0x4d);
ck_assert_int_eq(response.payload[4], 0x43);
ck_assert_int_eq(response.payload[5], 0x55);
ck_assert_int_eq(response.payload[6], 0x39);
ck_assert_int_eq(response.payload[7], 0x4a);
ck_assert_int_eq(response.payload[8], 0x39);
ck_assert_int_eq(response.payload[9], 0x34);
ck_assert_int_eq(response.payload[10], 0x48);
ck_assert_int_eq(response.payload[11], 0x55);
ck_assert_int_eq(response.payload[12], 0x41);
ck_assert_int_eq(response.payload[13], 0x30);
ck_assert_int_eq(response.payload[14], 0x34);
ck_assert_int_eq(response.payload[15], 0x35);
ck_assert_int_eq(response.payload[16], 0x32);
ck_assert_int_eq(response.payload[17], 0x34);
}
END_TEST

Suite* testSuite(void) {
Suite* s = suite_create("uds");
TCase *tc_core = tcase_create("core");
Expand All @@ -429,6 +500,7 @@ Suite* testSuite(void) {
tcase_add_test(tc_core, test_wrong_pid_then_right_completes);
tcase_add_test(tc_core, test_negative_response);
tcase_add_test(tc_core, test_payload_to_integer);
tcase_add_test(tc_core, test_response_multi_frame);

// TODO these are future work:
// TODO test request MIL
Expand Down

0 comments on commit 6e6ffb3

Please sign in to comment.