Skip to content

Commit

Permalink
ISO-TP fast: Remove typedefs for CAN ID and node address
Browse files Browse the repository at this point in the history
Rationale: They were not used consistently in the ThingSet CAN
subsystem and they hide the size of the data types.

Zephyr uses normal uint32_t for CAN IDs, so the ISO-TP fast should
do the same.

Also removing unused variables/functions.
  • Loading branch information
martinjaeger committed Dec 14, 2023
1 parent 631ffe5 commit 1741a23
Show file tree
Hide file tree
Showing 10 changed files with 55 additions and 69 deletions.
2 changes: 1 addition & 1 deletion include/thingset/can.h
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ struct thingset_can_request_response
{
struct k_sem sem;
struct k_timer timer;
isotp_fast_can_id can_id;
uint32_t can_id;
thingset_can_response_callback_t callback;
void *cb_arg;
};
Expand Down
17 changes: 6 additions & 11 deletions include/thingset/isotp_fast.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,6 @@
#define ISOTP_MSG_FDF BIT(3)
#endif

/* Represents a sender or a receiver in an ISO-TP fixed addressing scheme */
typedef uint8_t isotp_fast_node_id;
/* ISO-TP message CAN ID */
typedef uint32_t isotp_fast_can_id;

/**
* Callback invoked when a message is received.
*
Expand All @@ -28,8 +23,8 @@ typedef uint32_t isotp_fast_can_id;
* @param can_id The CAN ID of the message that has been received.
* @param arg The value of @ref recv_cb_arg passed to @ref isotp_fast_bind.
*/
typedef void (*isotp_fast_recv_callback_t)(struct net_buf *buffer, int rem_len,
isotp_fast_can_id can_id, void *arg);
typedef void (*isotp_fast_recv_callback_t)(struct net_buf *buffer, int rem_len, uint32_t can_id,
void *arg);

/**
* Callback invoked when an error occurs during message receiption.
Expand All @@ -38,7 +33,7 @@ typedef void (*isotp_fast_recv_callback_t)(struct net_buf *buffer, int rem_len,
* @param can_id The CAN ID of the sender of the message, if available.
* @param arg The value of @ref recv_cb_arg passed to @ref isotp_fast_bind.
*/
typedef void (*isotp_fast_recv_error_callback_t)(int8_t error, isotp_fast_can_id can_id, void *arg);
typedef void (*isotp_fast_recv_error_callback_t)(int8_t error, uint32_t can_id, void *arg);

/**
* Callback invoked when a message has been sent.
Expand Down Expand Up @@ -82,7 +77,7 @@ struct isotp_fast_ctx
/** Callback that is invoked when a message is sent */
isotp_fast_send_callback_t sent_callback;
/** CAN ID of this node, used in both transmission and receipt of messages */
isotp_fast_can_id rx_can_id;
uint32_t rx_can_id;
#ifdef CONFIG_ISOTP_FAST_BLOCKING_RECEIVE
sys_slist_t wait_recv_list;
#endif
Expand All @@ -107,7 +102,7 @@ struct isotp_fast_ctx
* @returns 0 on success, otherwise an error code < 0.
*/
int isotp_fast_bind(struct isotp_fast_ctx *ctx, const struct device *can_dev,
const isotp_fast_can_id rx_can_id, const struct isotp_fast_opts *opts,
const uint32_t rx_can_id, const struct isotp_fast_opts *opts,
isotp_fast_recv_callback_t recv_callback, void *recv_cb_arg,
isotp_fast_recv_error_callback_t recv_error_callback,
isotp_fast_send_callback_t sent_callback);
Expand Down Expand Up @@ -144,4 +139,4 @@ int isotp_fast_recv(struct isotp_fast_ctx *ctx, struct can_filter sender, uint8_
* @returns 0 on success.
*/
int isotp_fast_send(struct isotp_fast_ctx *ctx, const uint8_t *data, size_t len,
const isotp_fast_node_id target_addr, void *sent_cb_arg);
const uint8_t target_addr, void *sent_cb_arg);
11 changes: 5 additions & 6 deletions src/can.c
Original file line number Diff line number Diff line change
Expand Up @@ -436,8 +436,7 @@ int thingset_can_send_inst(struct thingset_can *ts_can, uint8_t *tx_buf, size_t
}
}

void isotp_fast_recv_callback(struct net_buf *buffer, int rem_len, isotp_fast_can_id can_id,
void *arg)
void isotp_fast_recv_callback(struct net_buf *buffer, int rem_len, uint32_t can_id, void *arg)
{
struct thingset_can *ts_can = arg;

Expand All @@ -460,7 +459,7 @@ void isotp_fast_recv_callback(struct net_buf *buffer, int rem_len, isotp_fast_ca
int tx_len =
thingset_process_message(&ts, ts_can->rx_buffer, len, sbuf->data, sbuf->size);
if (tx_len > 0) {
isotp_fast_node_id target_id = (uint8_t)(can_id & 0xFF);
uint8_t target_id = (uint8_t)(can_id & 0xFF);
int err = thingset_can_send_inst(ts_can, sbuf->data, tx_len, target_id, NULL, NULL,
K_NO_WAIT);
if (err != 0) {
Expand All @@ -474,7 +473,7 @@ void isotp_fast_recv_callback(struct net_buf *buffer, int rem_len, isotp_fast_ca
}
}

void isotp_fast_recv_error_callback(int8_t error, isotp_fast_can_id can_id, void *arg)
void isotp_fast_recv_error_callback(int8_t error, uint32_t can_id, void *arg)
{
LOG_ERR("RX error %d", error);
}
Expand Down Expand Up @@ -690,8 +689,8 @@ int thingset_can_init_inst(struct thingset_can *ts_can, const struct device *can
}

#ifdef CONFIG_ISOTP_FAST
isotp_fast_can_id rx_can_id = THINGSET_CAN_TYPE_CHANNEL | THINGSET_CAN_PRIO_CHANNEL
| THINGSET_CAN_TARGET_SET(ts_can->node_addr);
uint32_t rx_can_id = THINGSET_CAN_TYPE_CHANNEL | THINGSET_CAN_PRIO_CHANNEL
| THINGSET_CAN_TARGET_SET(ts_can->node_addr);
isotp_fast_bind(&ts_can->ctx, can_dev, rx_can_id, &fc_opts, isotp_fast_recv_callback, ts_can,
isotp_fast_recv_error_callback, isotp_fast_sent_callback);
#endif
Expand Down
24 changes: 12 additions & 12 deletions src/isotp_fast.c
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ NET_BUF_POOL_DEFINE(isotp_rx_pool,
CONFIG_ISOTP_FAST_RX_BUF_COUNT *CONFIG_ISOTP_FAST_RX_MAX_PACKET_COUNT,
CAN_MAX_DLEN - 1, sizeof(int), NULL);

static int get_send_ctx(struct isotp_fast_ctx *ctx, isotp_fast_can_id tx_can_id,
static int get_send_ctx(struct isotp_fast_ctx *ctx, uint32_t tx_can_id,
struct isotp_fast_send_ctx **sctx)
{
isotp_fast_node_id target_addr = isotp_fast_get_target_addr(tx_can_id);
uint8_t target_addr = isotp_fast_get_target_addr(tx_can_id);
struct isotp_fast_send_ctx *context;

SYS_SLIST_FOR_EACH_CONTAINER(&ctx->isotp_send_ctx_list, context, node)
Expand Down Expand Up @@ -105,10 +105,10 @@ static void free_recv_ctx_if_unowned(struct isotp_fast_recv_ctx *rctx)
free_recv_ctx(rctx);
}

static int get_recv_ctx(struct isotp_fast_ctx *ctx, isotp_fast_can_id rx_can_id,
static int get_recv_ctx(struct isotp_fast_ctx *ctx, uint32_t rx_can_id,
struct isotp_fast_recv_ctx **rctx)
{
isotp_fast_node_id source_addr = isotp_fast_get_source_addr(rx_can_id);
uint8_t source_addr = isotp_fast_get_source_addr(rx_can_id);
struct isotp_fast_recv_ctx *context;

SYS_SLIST_FOR_EACH_CONTAINER(&ctx->isotp_recv_ctx_list, context, node)
Expand Down Expand Up @@ -536,7 +536,7 @@ static void receive_can_rx(struct isotp_fast_recv_ctx *rctx, struct can_frame *f
}

static inline void prepare_frame(struct can_frame *frame, struct isotp_fast_ctx *ctx,
isotp_fast_can_id can_id)
uint32_t can_id)
{
frame->id = can_id;
frame->flags = CAN_FRAME_IDE | ((ctx->opts->flags & ISOTP_MSG_FDF) != 0 ? CAN_FRAME_FDF : 0);
Expand Down Expand Up @@ -616,7 +616,7 @@ static void can_rx_callback(const struct device *dev, struct can_frame *frame, v
{
struct isotp_fast_ctx *ctx = arg;
int index = 0;
isotp_fast_can_id can_id =
uint32_t can_id =
(frame->id & 0xFFFF0000) | ((frame->id & 0xFF00) >> 8) | ((frame->id & 0xFF) << 8);
if ((frame->data[index++] & ISOTP_PCI_TYPE_MASK) == ISOTP_PCI_TYPE_FC) {
LOG_DBG("Got flow control frame from %x", frame->id);
Expand Down Expand Up @@ -817,7 +817,7 @@ static void send_timeout_handler(struct k_timer *timer)
k_work_submit(&sctx->work);
}

static inline void prepare_filter(struct can_filter *filter, isotp_fast_can_id rx_can_id,
static inline void prepare_filter(struct can_filter *filter, uint32_t rx_can_id,
const struct isotp_fast_opts *opts)
{
filter->id = rx_can_id;
Expand All @@ -827,7 +827,7 @@ static inline void prepare_filter(struct can_filter *filter, isotp_fast_can_id r
}

int isotp_fast_bind(struct isotp_fast_ctx *ctx, const struct device *can_dev,
const isotp_fast_can_id rx_can_id, const struct isotp_fast_opts *opts,
const uint32_t rx_can_id, const struct isotp_fast_opts *opts,
isotp_fast_recv_callback_t recv_callback, void *recv_cb_arg,
isotp_fast_recv_error_callback_t recv_error_callback,
isotp_fast_send_callback_t sent_callback)
Expand Down Expand Up @@ -990,11 +990,11 @@ int isotp_fast_recv(struct isotp_fast_ctx *ctx, struct can_filter sender, uint8_
#endif /* CONFIG_ISOTP_FAST_BLOCKING_RECEIVE */

int isotp_fast_send(struct isotp_fast_ctx *ctx, const uint8_t *data, size_t len,
const isotp_fast_node_id target_addr, void *cb_arg)
const uint8_t target_addr, void *cb_arg)
{
const isotp_fast_can_id rx_can_id = (ctx->rx_can_id & 0xFFFF0000)
| (isotp_fast_get_target_addr(ctx->rx_can_id))
| (target_addr << ISOTP_FIXED_ADDR_TA_POS);
const uint32_t rx_can_id = (ctx->rx_can_id & 0xFFFF0000)
| (isotp_fast_get_target_addr(ctx->rx_can_id))
| (target_addr << ISOTP_FIXED_ADDR_TA_POS);
if (len <= (CAN_MAX_DLEN - ISOTP_FAST_SF_LEN_BYTE)) {
struct can_frame frame;
prepare_frame(&frame, ctx, rx_can_id);
Expand Down
25 changes: 10 additions & 15 deletions src/isotp_fast_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@
*/
struct isotp_fast_send_ctx
{
sys_snode_t node; /**< linked list node in @ref isotp_send_ctx_list */
struct isotp_fast_ctx *ctx; /**< pointer to bound context */
isotp_fast_can_id tx_can_id; /**< CAN ID used on sent message frames */
sys_snode_t node; /**< linked list node in @ref isotp_send_ctx_list */
struct isotp_fast_ctx *ctx; /**< pointer to bound context */
uint32_t tx_can_id; /**< CAN ID used on sent message frames */
struct k_work work;
struct k_timer timer; /**< handles timeouts */
struct k_sem sem; /**< used to ensure CF frames are sent in order */
Expand All @@ -55,9 +55,9 @@ struct isotp_fast_send_ctx
*/
struct isotp_fast_recv_ctx
{
sys_snode_t node; /**< linked list node in @ref isotp_recv_ctx_list */
struct isotp_fast_ctx *ctx; /**< pointer to bound context */
isotp_fast_can_id rx_can_id; /**< CAN ID on received frames */
sys_snode_t node; /**< linked list node in @ref isotp_recv_ctx_list */
struct isotp_fast_ctx *ctx; /**< pointer to bound context */
uint32_t rx_can_id; /**< CAN ID on received frames */
struct k_work work;
struct k_timer timer; /**< handles timeouts */
struct net_buf *buffer; /**< head node of buffer */
Expand Down Expand Up @@ -85,17 +85,12 @@ struct isotp_fast_recv_await_ctx
struct isotp_fast_recv_ctx *rctx;
};

static inline isotp_fast_node_id isotp_fast_get_source_addr(isotp_fast_can_id can_id)
static inline uint8_t isotp_fast_get_source_addr(uint32_t can_id)
{
return (isotp_fast_node_id)(can_id & ISOTP_FIXED_ADDR_SA_MASK);
return (uint8_t)(can_id & ISOTP_FIXED_ADDR_SA_MASK);
}

static inline isotp_fast_node_id isotp_fast_get_frame_source(struct can_frame *frame)
static inline uint8_t isotp_fast_get_target_addr(uint32_t can_id)
{
return (isotp_fast_node_id)(frame->id & ISOTP_FIXED_ADDR_SA_MASK);
}

static inline isotp_fast_node_id isotp_fast_get_target_addr(isotp_fast_can_id can_id)
{
return (isotp_fast_node_id)((can_id & ISOTP_FIXED_ADDR_TA_MASK) >> ISOTP_FIXED_ADDR_TA_POS);
return (uint8_t)((can_id & ISOTP_FIXED_ADDR_TA_MASK) >> ISOTP_FIXED_ADDR_TA_POS);
}
3 changes: 1 addition & 2 deletions tests/can/src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ uint8_t *response;
size_t response_len;
int response_code;

static void isotp_fast_recv_cb(struct net_buf *buffer, int rem_len, isotp_fast_can_id rx_can_id,
void *arg)
static void isotp_fast_recv_cb(struct net_buf *buffer, int rem_len, uint32_t rx_can_id, void *arg)
{
response = malloc(buffer->len);
memcpy(response, buffer->data, buffer->len);
Expand Down
5 changes: 2 additions & 3 deletions tests/isotp_fast/conformance/src/async_recv.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,7 @@ static int blocking_recv(uint8_t *buf, size_t size, k_timeout_t timeout)
return rx_len;
}

void isotp_fast_recv_handler(struct net_buf *buffer, int rem_len, isotp_fast_can_id rx_can_id,
void *arg)
void isotp_fast_recv_handler(struct net_buf *buffer, int rem_len, uint32_t rx_can_id, void *arg)
{
struct recv_msg msg = {
.len = buffer->len,
Expand All @@ -70,7 +69,7 @@ void isotp_fast_recv_handler(struct net_buf *buffer, int rem_len, isotp_fast_can
k_msgq_put(&recv_msgq, &msg, K_NO_WAIT);
}

void isotp_fast_recv_error_handler(int8_t error, isotp_fast_can_id rx_can_id, void *arg)
void isotp_fast_recv_error_handler(int8_t error, uint32_t rx_can_id, void *arg)
{
// printk("Error %d received\n", error);
recv_last_error = error;
Expand Down
25 changes: 13 additions & 12 deletions tests/isotp_fast/conformance/src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ static void send_sf(void)
{
int ret;

ret = isotp_fast_send(&ctx, random_data, DATA_SIZE_SF, rx_node_id, NULL);
ret = isotp_fast_send(&ctx, random_data, DATA_SIZE_SF, rx_node_addr, NULL);
zassert_equal(ret, 0, "Send returned %d", ret);
}

Expand Down Expand Up @@ -68,7 +68,7 @@ static void send_test_data(const uint8_t *data, size_t len)
{
int ret;

ret = isotp_fast_send(&ctx, data, len, rx_node_id, INT_TO_POINTER(ISOTP_N_OK));
ret = isotp_fast_send(&ctx, data, len, rx_node_addr, INT_TO_POINTER(ISOTP_N_OK));
zassert_equal(ret, 0, "Send returned %d", ret);
}

Expand Down Expand Up @@ -254,7 +254,8 @@ ZTEST(isotp_fast_conformance, test_send_sf_fixed)
filter_id = add_rx_msgq(tx_can_id, CAN_EXT_ID_MASK);
zassert_true((filter_id >= 0), "Negative filter number [%d]", filter_id);

ret = isotp_fast_send(&ctx, random_data, DATA_SIZE_SF, rx_node_id, INT_TO_POINTER(ISOTP_N_OK));
ret =
isotp_fast_send(&ctx, random_data, DATA_SIZE_SF, rx_node_addr, INT_TO_POINTER(ISOTP_N_OK));
zassert_equal(ret, 0, "Send returned %d", ret);

check_frame_series(&des_frame, 1, &frame_msgq);
Expand Down Expand Up @@ -486,7 +487,7 @@ ZTEST(isotp_fast_conformance, test_send_timeouts)
/* Test timeout for first FC*/
k_sem_reset(&send_compl_sem);
start_time = k_uptime_get_32();
isotp_fast_send(&ctx, random_data, sizeof(random_data), rx_node_id,
isotp_fast_send(&ctx, random_data, sizeof(random_data), rx_node_addr,
INT_TO_POINTER(ISOTP_N_TIMEOUT_BS));
ret = k_sem_take(&send_compl_sem, K_MSEC(BS_TIMEOUT_UPPER_MS));
time_diff = k_uptime_get_32() - start_time;
Expand All @@ -495,7 +496,7 @@ ZTEST(isotp_fast_conformance, test_send_timeouts)

/* Test timeout for consecutive FC frames */
k_sem_reset(&send_compl_sem);
ret = isotp_fast_send(&ctx, random_data, sizeof(random_data), rx_node_id,
ret = isotp_fast_send(&ctx, random_data, sizeof(random_data), rx_node_addr,
INT_TO_POINTER(ISOTP_N_TIMEOUT_BS));
zassert_equal(ret, ISOTP_N_OK, "Send returned %d", ret);

Expand All @@ -510,7 +511,7 @@ ZTEST(isotp_fast_conformance, test_send_timeouts)

/* Test timeout reset with WAIT frame */
k_sem_reset(&send_compl_sem);
ret = isotp_fast_send(&ctx, random_data, sizeof(random_data), rx_node_id,
ret = isotp_fast_send(&ctx, random_data, sizeof(random_data), rx_node_addr,
INT_TO_POINTER(ISOTP_N_TIMEOUT_BS));
zassert_equal(ret, ISOTP_N_OK, "Send returned %d", ret);

Expand Down Expand Up @@ -666,7 +667,7 @@ ZTEST(isotp_fast_conformance, test_sender_fc_errors)
fc_frame.length = DATA_SIZE_FC;

k_sem_reset(&send_compl_sem);
ret = isotp_fast_send(&ctx, random_data, DATA_SEND_LENGTH, rx_node_id,
ret = isotp_fast_send(&ctx, random_data, DATA_SEND_LENGTH, rx_node_addr,
INT_TO_POINTER(ISOTP_N_INVALID_FS));
zassert_equal(ret, ISOTP_N_OK, "Send returned %d", ret);

Expand All @@ -678,12 +679,12 @@ ZTEST(isotp_fast_conformance, test_sender_fc_errors)
/* buffer overflow */
can_remove_rx_filter(can_dev, filter_id);

ret = isotp_fast_send(&ctx, random_data, 5 * 1024, rx_node_id, NULL);
ret = isotp_fast_send(&ctx, random_data, 5 * 1024, rx_node_addr, NULL);
zassert_equal(ret, ISOTP_N_BUFFER_OVERFLW, "Expected overflow but got %d", ret);
filter_id = add_rx_msgq(tx_can_id, CAN_EXT_ID_MASK);

k_sem_reset(&send_compl_sem);
ret = isotp_fast_send(&ctx, random_data, DATA_SEND_LENGTH, rx_node_id,
ret = isotp_fast_send(&ctx, random_data, DATA_SEND_LENGTH, rx_node_addr,
INT_TO_POINTER(ISOTP_N_BUFFER_OVERFLW));

check_frame_series(&ff_frame, 1, &frame_msgq);
Expand All @@ -694,7 +695,7 @@ ZTEST(isotp_fast_conformance, test_sender_fc_errors)

/* wft overrun */
k_sem_reset(&send_compl_sem);
ret = isotp_fast_send(&ctx, random_data, DATA_SEND_LENGTH, rx_node_id,
ret = isotp_fast_send(&ctx, random_data, DATA_SEND_LENGTH, rx_node_addr,
INT_TO_POINTER(ISOTP_N_WFT_OVRN));

check_frame_series(&ff_frame, 1, &frame_msgq);
Expand All @@ -721,7 +722,7 @@ ZTEST(isotp_fast_conformance, test_sf_length)
filter_id = add_rx_msgq(tx_can_id, CAN_EXT_ID_MASK);
zassert_true((filter_id >= 0), "Negative filter number [%d]", filter_id);

ret = isotp_fast_send(&ctx, random_data, 7, rx_node_id, INT_TO_POINTER(ISOTP_N_OK));
ret = isotp_fast_send(&ctx, random_data, 7, rx_node_addr, INT_TO_POINTER(ISOTP_N_OK));
zassert_equal(ret, 0, "Send returned %d", ret);

check_frame_series(&des_frame, 1, &frame_msgq);
Expand All @@ -731,7 +732,7 @@ ZTEST(isotp_fast_conformance, test_sf_length)
memcpy(&des_frame.data[2], random_data, DATA_SIZE_SF);
des_frame.length = 9;

ret = isotp_fast_send(&ctx, random_data, 9, rx_node_id, INT_TO_POINTER(ISOTP_N_OK));
ret = isotp_fast_send(&ctx, random_data, 9, rx_node_addr, INT_TO_POINTER(ISOTP_N_OK));
zassert_equal(ret, 0, "Send returned %d", ret);

check_frame_series(&des_frame, 1, &frame_msgq);
Expand Down
7 changes: 3 additions & 4 deletions tests/isotp_fast/conformance/src/main.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,10 @@ const struct isotp_fast_opts fc_opts = {
#endif
};

const isotp_fast_can_id rx_can_id = 0x18DA0201;
const isotp_fast_can_id tx_can_id = 0x18DA0102;
const uint32_t rx_can_id = 0x18DA0201;
const uint32_t tx_can_id = 0x18DA0102;

const isotp_fast_node_id rx_node_id = 0x01;
const isotp_fast_node_id tx_node_id = 0x02;
const uint8_t rx_node_addr = 0x01;

const struct device *const can_dev = DEVICE_DT_GET(DT_CHOSEN(zephyr_canbus));
struct isotp_fast_ctx ctx;
Expand Down
5 changes: 2 additions & 3 deletions tests/isotp_fast/conformance/src/sync_recv.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,8 @@ static int blocking_recv(uint8_t *buf, size_t size, k_timeout_t timeout)
return isotp_fast_recv(&ctx, sender, buf, size, timeout);
}

void isotp_fast_recv_handler(struct net_buf *buffer, int rem_len, isotp_fast_can_id can_id,
void *arg)
void isotp_fast_recv_handler(struct net_buf *buffer, int rem_len, uint32_t can_id, void *arg)
{}

void isotp_fast_recv_error_handler(int8_t error, isotp_fast_can_id can_id, void *arg)
void isotp_fast_recv_error_handler(int8_t error, uint32_t can_id, void *arg)
{}

0 comments on commit 1741a23

Please sign in to comment.