paoloteti/libj1939

Library to work with J1939 Frames used in CAN bus in car and trucks industries

libj1939's Issues

pointer to not initialized variable passed as parameter to function

Local variable pgn is passed frm pgn_pool_receive() to j1939_receive not initialized, but used in j1939_pdu_is_p2p2()

int pgn_pool_receive(void)
{
struct hasht_entry *entry;
### j1939_pgn_t pgn;
uint8_t code, src, priority, dest;
uint32_t len;
uint8_t data[8];
int ret;

ret = j1939_receive(### &pgn, &priority, &src, &dest, data, &len);

int j1939_receive(j1939_pgn_t### *pgn, uint8_t *priority, uint8_t *src,
uint8_t *dst, uint8_t *data, uint32_t *len)
{
uint32_t id;
int received;

if (unlikely(### !pgn || !priority || !src || !dst || !data || !len)) {
	return -1;
}
received = j1939_canrcv(&id, data);
if (received >= 0) {
	*len = received;
	*priority = (id & 0x1C000000u) >> 26;
	*src = id & 0x000000FFu;

	/*
	 * if PGN is peer-to-peer, remove destination from
	 * PGN itself and calculate destination address
	 */
	**if (j1939_pdu_is_p2p2(### *pgn)) {**

CAN Filter

I apologize for using this communication path, but I don't feel comfortable doing a pull request.
I am using libj1939 a in an application and I am using filters. I ran into a problem and found a solution. It may be appropriate to include this solution in the library. The method in question is j1939_filter in linux_socketcan.c.

int j1939_filter(struct j1939_pgn_filter *filter, uint32_t num_filters)
{
struct can_filter rfilter[num_filters];
uint32_t id;

for (size_t i = 0; i < num_filters; i++) {
	id = j1939_pgn2id(filter[i].pgn, filter[i].priority, filter[i].addr);

    // all of the network messages have the CAN_EFF_FLAG bit cleared, so I don't
    // understand why this is set here. The mask is zero, so this bit wouldn't
    // be considered in the filter anyway
	rfilter[i].can_id = id | CAN_EFF_FLAG;
    
    // the filter mask didn't line up with the ID so I used the pgn2id macro to align it
    // I didn't know what to do with the polarity bit, so I left it at 0
    // I did not need to filter on the polarity
	// original code - rfilter[i].can_mask = filter[i].pgn_mask;
    rfilter[i].can_mask = j1939_pgn2id(filter[i].pgn_mask, 0, filter[i].addr_mask);

    // test code to verify the id and mask
	//printf("ID: %08x, Mask: %08x\n", rfilter[i].can_id, rfilter[i].can_mask);
}
setsockopt(cansock, SOL_CAN_RAW, CAN_RAW_FILTER, &rfilter, sizeof(rfilter));

return 0;

}

Thanks for your consideration.

Karl Kobel

BTW - we have a slight connection - I work for Wabtec (albeit in the bus division). I know we have done work for Alstom.

[email protected]

Wrong handling of PGN in j1939_receive()

if (j1939_pdu_is_p2p(*pgn)) {
*pgn = id & 0xFFFF00FFu;
*dst = (id >> 8) & 0x000000FFu;
} else {
*dst = ADDRESS_NULL; <-----*pgn = ???
}
*pgn = (*pgn >> 8) & PGN_MASK;

Bad endian in size calc

In eom_ack_send()

uint8_t data[DLC_MAX] = {
CONN_MODE_EOM_ACK,
size & 0x00FF,
(size >> 8),

In eom_ack_recevied():

uint8_t eom_ack_size = (data[1] << 8) | (data[2]);

EOM ACK sending transmitting node

Isn't it supposed to be, that DT messages are acknowledged by receiver (in the source code done way around from send_tp()