devcoons / iso15765-canbus

Implementation of ISO15765-2 in C
MIT License
140 stars 60 forks source link

[Question] How interact with hardware #16

Closed juicebox-evse closed 1 year ago

juicebox-evse commented 1 year ago

Hello,

I implement this library in my small project. I have custom PCB with rp pico and CAN transceiver.

I need create frames and send it as bytes via UART to onboard CAN transceiver. I added method for sending bytes to UART in send_frame callback function. But my question is how I can receive some data from UART? I need read incoming bytes from UART and assemble canbus_frame_t from it. I thought that usda_indication() is for that, but I dont know how invoke it from my UART reader.

Is that possible?

Thank you!

juicebox-evse commented 1 year ago

I solved it myself, I omitted iso15765_enqueue() in description, now it is working without problem, I am able send and receive packets.

Thank you anyway.

mitkodev commented 10 months ago

hey @juicebox-evse, i think i have the exact same question as you, can you please check my logic and tell me if it is the correct flow. I am trying to just send a simple OBD2 command to read the rpm. So first:

n_req_t frame =
{
    .n_ai.n_pr = 0x06,      // Network Address Priority
    .n_ai.n_sa = 0x7DF,     // Network Source Address
    .n_ai.n_ta = 0x7E8,     // Network Target Address
    .n_ai.n_ae = 0x00,      // Network Address Extension
    .n_ai.n_tt = N_TA_T_PHY,    // Network Target Address type
    .fr_fmt = CBUS_FR_FRM_STD,  // CANFD or CANSTD
    .msg = {0x02,0x01,0x0C},
    .msg_sz = 3,        // Message size
};
iso15765_send(&handler, &frame);

Then I get to send_frame so:

static uint8_t send_frame(cbus_id_type id_type, uint32_t id, cbus_fr_format fr_fmt, uint8_t dlc, uint8_t* dt)
{
        // my driver 
        // CAN_TransmitMessage
        // CAN_ReceiveMessage
        // Transform my received message to canbus_frame_t  and enqueue
        canbus_frame_t frame = { .id = 0x7E8, .dlc = 8, .id_type = CBUS_ID_T_STANDARD, .fr_format= CBUS_FR_FRM_STD, .dt = { 0x04, 0x41, 0x0C, 0x0B, 0x68, 0xCC, 0xCC, 0xCC}};
        iso15765_enqueue(&handler, &frame);
        return 0;
}

And then I should be getting something in usdata_indication callback (Unfortunately I dont)? Is my approach correct or am I missing something? Thanks!

devcoons commented 10 months ago

hey @juicebox-evse, i think i have the exact same question as you, can you please check my logic and tell me if it is the correct flow. I am trying to just send a simple OBD2 command to read the rpm. So first:

n_req_t frame =
{
    .n_ai.n_pr = 0x06,        // Network Address Priority
    .n_ai.n_sa = 0x7DF,       // Network Source Address
    .n_ai.n_ta = 0x7E8,       // Network Target Address
    .n_ai.n_ae = 0x00,        // Network Address Extension
    .n_ai.n_tt = N_TA_T_PHY,  // Network Target Address type
    .fr_fmt = CBUS_FR_FRM_STD,    // CANFD or CANSTD
    .msg = {0x02,0x01,0x0C},
    .msg_sz = 3,      // Message size
};
iso15765_send(&handler, &frame);

Then I get to send_frame so:

static uint8_t send_frame(cbus_id_type id_type, uint32_t id, cbus_fr_format fr_fmt, uint8_t dlc, uint8_t* dt)
{
        // my driver 
        // CAN_TransmitMessage
        // CAN_ReceiveMessage
        // Transform my received message to canbus_frame_t  and enqueue
        canbus_frame_t frame = { .id = 0x7E8, .dlc = 8, .id_type = CBUS_ID_T_STANDARD, .fr_format= CBUS_FR_FRM_STD, .dt = { 0x04, 0x41, 0x0C, 0x0B, 0x68, 0xCC, 0xCC, 0xCC}};
        iso15765_enqueue(&handler, &frame);
        return 0;
}

And then I should be getting something in usdata_indication callback (Unfortunately I dont)? Is my approach correct or am I missing something? Thanks!

Hi! The first part seems to be fine (as long as you are sure about the addresses and the message you want to send. However, in the send function you should probably use the CAN transmission function of your HAL (the api of you microcontroller). After verifying that, you need to 'enqueue' any incoming frame by placing the following code in your HAL CAN receive callback

....
canbus_frame_t frame = { .id = id, .dlc = dlc, .id_type = id_type, .fr_format= fr_fmt };
        memmove(frame.dt, dt, dlc);
        iso15765_enqueue(&handler, &frame);
....

Please share some more info to help you on it.

mitkodev commented 10 months ago

hey @devcoons, thank you for the info, i followed your advice and after some debugging it turns out the enqueue function is doing nothing, because it does not pass

if (_queue->first != _queue->next)

I must add that i am trying to compile this with armcc and I am getting errors in the libqueue library about incompatible pointers here https://github.com/devcoons/iso15765-canbus/blob/4233d5f73866dd866ffef922fc50f05ea20c0212/lib/lib_iqueue.c#L62C28-L63C2. When I try to compile it with gcc I only get warning, not error. However by trying to fix and do casting between these pointers, i might be breaking the queue itself and that's why I cannot enqueue new message and I cannot get my callback fired. I also get error about the ALIGNMENT macro that cannot be used, so in the end I think the problem is entirely on my end :)

devcoons commented 10 months ago

hey @devcoons, thank you for the info, i followed your advice and after some debugging it turns out the enqueue function is doing nothing, because it does not pass

if (_queue->first != _queue->next)

I must add that i am trying to compile this with armcc and I am getting errors in the libqueue library about incompatible pointers here https://github.com/devcoons/iso15765-canbus/blob/4233d5f73866dd866ffef922fc50f05ea20c0212/lib/lib_iqueue.c#L62C28-L63C2. When I try to compile it with gcc I only get warning, not error. However by trying to fix and do casting between these pointers, i might be breaking the queue itself and that's why I cannot enqueue new message and I cannot get my callback fired. I also get error about the ALIGNMENT macro that cannot be used, so in the end I think the problem is entirely on my end :)

Hi @mitkodev, i've made a small update in the i_queue library. Please let me know if now it compiles also in armcc. Regarding the ALIGNMENT, i saw that armcc do support the same gcc macro https://developer.arm.com/documentation/dui0472/m/Compiler-specific-Features/--attribute----aligned---variable-attribute?lang=en , but i'll have a deeper look on that.

Can you please confirm that you do more or less the following?

static iso15765_t handler1 =
{
        .addr_md = N_ADM_NORMAL,
        .fr_id_type = CBUS_ID_T_STANDARD,
        .clbs.send_frame = send_frame1,
        .clbs.on_error = on_error,
        .clbs.get_ms = getms,
        .clbs.indn = indn1,
        .config.stmin = 0x3,
        .config.bs = 0x0f,
        .config.n_bs = 100,
        .config.n_cr = 3
};

static uint32_t getms()
{
        return GetTickCount(); // Replace this with your sys tick (the function requires ms)
}

static void on_error(n_rslt err_type)
{
      //  printf("ERROR OCCURED!:%04x", err_type);  //
}

static void indn1(n_indn_t* info)
{
   // Here you will get the ISO TP reply
}

static uint8_t send_frame1(cbus_id_type id_type, uint32_t id, cbus_fr_format fr_fmt, uint8_t dlc, uint8_t* dt)
{
        // Here use your hal can library to transmit the frame 
        return 0;
}

...
...
...

// Put the following code in your HAL CAN Receive Callback (modify appropriately)
{
        canbus_frame_t frame = { .id = id, .dlc = dlc, .id_type = id_type,  .fr_format = fr_fmt };
        memmove(frame.dt, dt, dlc);
        iso15765_enqueue(&handler1, &frame);
}

....
....
....

....
....
....

// Run this just once in your main function
 iso15765_init(&handler1);

...
...
...

// Your ISO TP transmit message

n_req_t frame1 =
{
    .n_ai.n_pr = 0x06,      // Network Address Priority
    .n_ai.n_sa = 0x7DF,     // Network Source Address
    .n_ai.n_ta = 0x7E8,     // Network Target Address
    .n_ai.n_ae = 0x00,      // Network Address Extension
    .n_ai.n_tt = N_TA_T_PHY,    // Network Target Address type
    .fr_fmt = CBUS_FR_FRM_STD,  // CANFD or CANSTD
    .msg = {0x02,0x01,0x0C},
    .msg_sz = 3,        // Message size
};

iso15765_send(&handler1, &frame1);

....
....
....

// Call this in a loop to process the outcoming/incoming messages. 
iso15765_process(&handler1);
mitkodev commented 10 months ago

@devcoons Thank you very much, now the logic is correct and working!! I can send and receive message, especially multipart like the VIN is also working!

I have one small question if this is the intented behaviour: I have iso15765_processin a separate thread running every 100ms. In my main thread I iso15765_send two can messages with same source and target address (basically reading OBD2 rpm and speed) every second, but only the message that I iso15765_send first is being sent on the CAN line. Should I wait somehow for process to send the first message before trying to send new message?

Thanks again for the help!

devcoons commented 10 months ago

@devcoons Thank you very much, now the logic is correct and working!! I can send and receive message, especially multipart like the VIN is also working!

I have one small question if this is the intented behaviour: I have iso15765_processin a separate thread running every 100ms. In my main thread I iso15765_send two can messages with same source and target address (basically reading OBD2 rpm and speed) every second, but only the message that I iso15765_send first is being sent on the CAN line. Should I wait somehow for process to send the first message before trying to send new message?

Thanks again for the help!

Hi @mitkodev! Happy to hear that now it works.

As a suggestion, try to increase speed of the iso15765_process thread. Keep in mind that this function is actually calling the CAN frame transmission. I usually put this in a normal-low priority thread with a 2ms delay. Especially if you are going to transmit multiframes it requires a fast loop. To help you understand a bit better, think that every time this function gets called it decides to transmit or not a canbus frame (+ if there is an ISO-TP frame to fire the callback etc..) so in case of a multiframe, this function will be required to run multiple times to transmit the complete ISO-TP frame.

Regarding the send, it suppose to send just one ISO-TP per time. So i suppose that if you increase the speed of the thread i've mentioned before, both of your send commands will be successfully executed.

Let me know when you have any results

mitkodev commented 10 months ago

@devcoons Reduced polling to 5ms, added some 50ms timeout after sending a CAN command and now everything is working as expected now, thank you for the help!

mitkodev commented 8 months ago

hey @devcoons i dont want to open new thread cause it't just a simple question :) in your opinion during the process of enqueued can_frame, is there some data operation that might cause UNALLIGNED memory access (cortex M4)? Very rarely I get hard fault right after receiving new CAN interrupt with this info: [MAIN] SCB->BFAR = 0xe000ed38 [MAIN] SCB->MMFAR = 0xe000ed34 [MAIN] SCB->HFSR = 0x40000000 [MAIN] SCB->CFSR = 0x01000400 [MAIN] r0 = 0x2001ffff [MAIN] r1 = 0x20000cdc [MAIN] r2 = 0x00000000 [MAIN] r3 = 0xdeaddead [MAIN] r12 = 0x12121212 [MAIN] lr = 0xfffffffd [MAIN] pc = 0x0802028a [MAIN] psr = 0x6100000e

and
0x08020284: 680a .h LDR r2,[r1,#0] 0x08020286: 6002 .` STR r2,[r0,#0] 0x08020288: 6810 .h LDR r0,[r2,#0] 0x0802028a: e8900ff0 .... LDM r0,{r4-r11}

which is cause by the LDM instruction according to internet. I made sure that even canbus_frame_t use ALIGNMENT attribute, but still happens from time to time.

devcoons commented 8 months ago

hey @devcoons i dont want to open new thread cause it't just a simple question :) in your opinion during the process of enqueued can_frame, is there some data operation that might cause UNALLIGNED memory access (cortex M4)? Very rarely I get hard fault right after receiving new CAN interrupt with this info: [MAIN] SCB->BFAR = 0xe000ed38 [MAIN] SCB->MMFAR = 0xe000ed34 [MAIN] SCB->HFSR = 0x40000000 [MAIN] SCB->CFSR = 0x01000400 [MAIN] r0 = 0x2001ffff [MAIN] r1 = 0x20000cdc [MAIN] r2 = 0x00000000 [MAIN] r3 = 0xdeaddead [MAIN] r12 = 0x12121212 [MAIN] lr = 0xfffffffd [MAIN] pc = 0x0802028a [MAIN] psr = 0x6100000e

and
0x08020284: 680a .h LDR r2,[r1,#0] 0x08020286: 6002 .` STR r2,[r0,#0] 0x08020288: 6810 .h LDR r0,[r2,#0] 0x0802028a: e8900ff0 .... LDM r0,{r4-r11}

which is cause by the LDM instruction according to internet. I made sure that even canbus_frame_t use ALIGNMENT attribute, but still happens from time to time.

Hi @mitkodev ! As long as this is happening only during the interrupt callback, the problem could be in iqueue enqueue function. Can you reduce the I15765_QUEUE_ELMS from 64 to 8 or 16 and check if this fault is happening more often?