devcoons / iso15765-canbus Goto Github PK

Hi, how to implement in STM32 using HAL?

First and foremost, I would like to thank you for the excellent work you have done !
This is my first ever issue on an open source project so feel free to let me know if I've done things incorrectly.

I found a small bug concerning operator precedence in the function n_pdu_unpack().
When doing (uint8_t)(id & 0x0000FF00) >> 8 and similar operations, the type cast has precedence over the bitshift operation. (See cpprefence and MS C)
This results in the value being casted to an uint8_t with a value of zero before the bitshift which is incorrect.

Just adding an extra set of parentheses around the bit shift operation does the job:

        case N_ADM_MIXED29:
                n_pdu->n_ai.n_ae = dt[0];
-               n_pdu->n_ai.n_pr = (uint8_t)(id & 0x1C000000) >> 26;
+               n_pdu->n_ai.n_pr = (uint8_t)((id & 0x1C000000) >> 26);
                n_pdu->n_ai.n_tt = (uint8_t)(((id & 0x00FF0000) >> 16) == 0xCE ? N_TA_T_PHY : N_TA_T_FUNC);
-               n_pdu->n_ai.n_ta = (uint8_t)(id & 0x0000FF00) >> 8;
-               n_pdu->n_ai.n_sa = (uint8_t)(id & 0x000000FF) >> 0;
+               n_pdu->n_ai.n_ta = (uint8_t)((id & 0x0000FF00) >> 8);
+               n_pdu->n_ai.n_sa = (uint8_t)((id & 0x000000FF) >> 0);
                break;

I tested the fix against pylessard's can-isotp. Before the fix, the two libraries had issues talking to each other and now it works just fine.
I am fairly confident about applying the fix to N_ADM_MIXED29 and N_ADM_FIXED addressing modes but I am not so sure about N_ADM_NORMAL and N_ADM_EXTENDED since the correspondence between N_AI and the CAN ID is left open in those modes.

Do you want me to fork and open a short PR ? If so, do I also add an extra set of parentheses in the N_ADM_NORMAL and N_ADM_EXTENDED cases ?

Thank you for you time and, again, thank you for your work !

the first parameter must be ih->can_md, but ih->addr_md used

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!

Hi
Thank you for the great work.

In function lib_iso15765.c::process_in_cf there is a return of N_UNE_PDU when last continuos frame is received and correctly signalled to the upper levels. Is it correct?

	if (ih->in.msg_pos >= ih->in.msg_sz)
	{
		signaling(N_INDN, &ih->in, (void*)ih->clbs.indn, ih->in.msg_sz, N_OK);
		memset(&ih->in, 0, sizeof(n_iostream_t));
		ih->in.last_upd.n_cr = 0;
		//	ih->in.sts = N_S_IDLE;
		return N_UNE_PDU;
	}

Thank you very much

The following code may be wrong.

1. lib_iso15765.c // sequence issue
   /* Increase the CF counter and check if the reception sequence is ok */
   ih->in.cf_cnt = ih->in.cf_cnt + 1 > 0x0F ? 1 : ih->in.cf_cnt + 1;
   =>
   ih->in.cf_cnt = ih->in.cf_cnt + 1 > 0x0F ? 0 : ih->in.cf_cnt + 1;

2. lib_iso15765.c // reach max counter issue
   /* if we reach the max CF counter, then we send a FC frame */
   ...
   if (ih->in.cf_cnt % ih->config.bs)
   =>
   if ((ih->in.cf_cnt % ih->config.bs) == 0)

I think this piece should not be N_ADM_EXTENDED, but N_ADM_FIXED.
And when defining the enumeration of this block, the two comments FIX and EXTEN are mixed up.

How would simultaneous multi-node communication be handled?
Should I just create multiple handlers and then call the correct handler on the correct incoming frames based on arbitration ID?

What is the intended solution for sending flow control frames on a received first frame? I haven't found any option to configure on which IDs it should reply.
Or is it intended to implement it manually in usdata_indication?