From f13791d5baf2edb6263f7a484b4faa77df1e2997 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20J=C3=A4ger?= Date: Wed, 30 Aug 2023 16:14:19 +0200 Subject: [PATCH] canbus: isotp: use flags for configuration in isotp_msg_id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous design with dedicated bits in the structure required a user to explicitly set each bit. Using one flags variable allows to extend the features more easily in the future and avoids breaking existing code. This change is particularly useful for the FDF and BRS flags required for CAN-FD support. See also previous similar change for the CAN driver in f8a88cdb2791fe40b3021f1275fe250c2c17dcf3 Signed-off-by: Martin Jäger --- include/zephyr/canbus/isotp.h | 29 ++++++++++++---- samples/subsys/canbus/isotp/src/main.c | 8 ----- subsys/canbus/isotp/isotp.c | 34 ++++++++++--------- .../canbus/isotp/conformance/src/main.c | 23 ++++--------- .../canbus/isotp/implementation/src/main.c | 5 +-- 5 files changed, 49 insertions(+), 50 deletions(-) diff --git a/include/zephyr/canbus/isotp.h b/include/zephyr/canbus/isotp.h index 6ce32c48edb..b975837915f 100644 --- a/include/zephyr/canbus/isotp.h +++ b/include/zephyr/canbus/isotp.h @@ -128,6 +128,27 @@ extern "C" { #endif +/** + * @name ISO-TP message ID flags + * @anchor ISOTP_MSG_FLAGS + * + * @{ + */ + +/** Message uses ISO-TP extended addressing (first payload byte of CAN frame) */ +#define ISOTP_MSG_EXT_ADDR BIT(0) + +/** + * Message uses ISO-TP fixed addressing (according to SAE J1939). Only valid in combination with + * ``ISOTP_MSG_IDE``. + */ +#define ISOTP_MSG_FIXED_ADDR BIT(1) + +/** Message uses extended (29-bit) CAN ID */ +#define ISOTP_MSG_IDE BIT(2) + +/** @} */ + /** * @brief ISO-TP message id struct * @@ -146,12 +167,8 @@ struct isotp_msg_id { }; /** ISO-TP extended address (if used) */ uint8_t ext_addr; - /** Indicates the CAN identifier type (0 for standard or 1 for extended) */ - uint8_t ide : 1; - /** Indicates if ISO-TP extended addressing is used */ - uint8_t use_ext_addr : 1; - /** Indicates if ISO-TP fixed addressing (acc. to SAE J1939) is used */ - uint8_t use_fixed_addr : 1; + /** Flags. @see @ref ISOTP_MSG_FLAGS. */ + uint8_t flags; }; /* diff --git a/samples/subsys/canbus/isotp/src/main.c b/samples/subsys/canbus/isotp/src/main.c index 8fbd43e28ac..0a37654fd3d 100644 --- a/samples/subsys/canbus/isotp/src/main.c +++ b/samples/subsys/canbus/isotp/src/main.c @@ -12,23 +12,15 @@ const struct isotp_fc_opts fc_opts_0_5 = {.bs = 0, .stmin = 5}; const struct isotp_msg_id rx_addr_8_0 = { .std_id = 0x80, - .ide = 0, - .use_ext_addr = 0 }; const struct isotp_msg_id tx_addr_8_0 = { .std_id = 0x180, - .ide = 0, - .use_ext_addr = 0 }; const struct isotp_msg_id rx_addr_0_5 = { .std_id = 0x01, - .ide = 0, - .use_ext_addr = 0 }; const struct isotp_msg_id tx_addr_0_5 = { .std_id = 0x101, - .ide = 0, - .use_ext_addr = 0 }; const struct device *can_dev; diff --git a/subsys/canbus/isotp/isotp.c b/subsys/canbus/isotp/isotp.c index 85b28ebe1f7..4a55deb76db 100644 --- a/subsys/canbus/isotp/isotp.c +++ b/subsys/canbus/isotp/isotp.c @@ -118,7 +118,7 @@ static inline uint32_t receive_get_sf_length(struct net_buf *buf) static void receive_send_fc(struct isotp_recv_ctx *ctx, uint8_t fs) { struct can_frame frame = { - .flags = ctx->tx_addr.ide != 0 ? CAN_FRAME_IDE : 0, + .flags = (ctx->tx_addr.flags & ISOTP_MSG_IDE) != 0 ? CAN_FRAME_IDE : 0, .id = ctx->tx_addr.ext_id }; uint8_t *data = frame.data; @@ -127,7 +127,7 @@ static void receive_send_fc(struct isotp_recv_ctx *ctx, uint8_t fs) __ASSERT_NO_MSG(!(fs & ISOTP_PCI_TYPE_MASK)); - if (ctx->tx_addr.use_ext_addr) { + if ((ctx->tx_addr.flags & ISOTP_MSG_EXT_ADDR) != 0) { *data++ = ctx->tx_addr.ext_addr; } @@ -374,13 +374,13 @@ static void process_ff_sf(struct isotp_recv_ctx *ctx, struct can_frame *frame) uint8_t payload_len; uint32_t rx_sa; /* ISO-TP fixed source address (if used) */ - if (ctx->rx_addr.use_ext_addr) { + if ((ctx->rx_addr.flags & ISOTP_MSG_EXT_ADDR) != 0) { if (frame->data[index++] != ctx->rx_addr.ext_addr) { return; } } - if (ctx->rx_addr.use_fixed_addr) { + if ((ctx->rx_addr.flags & ISOTP_MSG_FIXED_ADDR) != 0) { /* store actual CAN ID used by the sender */ ctx->rx_addr.ext_id = frame->id; /* replace TX target address with RX source address */ @@ -465,7 +465,7 @@ static void process_cf(struct isotp_recv_ctx *ctx, struct can_frame *frame) int index = 0; uint32_t data_len; - if (ctx->rx_addr.use_ext_addr) { + if ((ctx->rx_addr.flags & ISOTP_MSG_EXT_ADDR) != 0) { if (frame->data[index++] != ctx->rx_addr.ext_addr) { return; } @@ -557,14 +557,15 @@ static inline int attach_ff_filter(struct isotp_recv_ctx *ctx) { uint32_t mask; - if (ctx->rx_addr.use_fixed_addr) { + if ((ctx->rx_addr.flags & ISOTP_MSG_FIXED_ADDR) != 0) { mask = ISOTP_FIXED_ADDR_RX_MASK; } else { mask = CAN_EXT_ID_MASK; } struct can_filter filter = { - .flags = CAN_FILTER_DATA | ((ctx->rx_addr.ide != 0) ? CAN_FILTER_IDE : 0), + .flags = CAN_FILTER_DATA | + ((ctx->rx_addr.flags & ISOTP_MSG_IDE) != 0 ? CAN_FILTER_IDE : 0), .id = ctx->rx_addr.ext_id, .mask = mask }; @@ -755,7 +756,7 @@ static void send_process_fc(struct isotp_send_ctx *ctx, { uint8_t *data = frame->data; - if (ctx->rx_addr.use_ext_addr) { + if ((ctx->rx_addr.flags & ISOTP_MSG_EXT_ADDR) != 0) { if (ctx->rx_addr.ext_addr != *data++) { return; } @@ -854,7 +855,7 @@ static void pull_data_ctx(struct isotp_send_ctx *ctx, size_t len) static inline int send_sf(struct isotp_send_ctx *ctx) { struct can_frame frame = { - .flags = ctx->tx_addr.ide != 0 ? CAN_FRAME_IDE : 0, + .flags = (ctx->tx_addr.flags & ISOTP_MSG_IDE) != 0 ? CAN_FRAME_IDE : 0, .id = ctx->tx_addr.ext_id }; size_t len = get_ctx_data_length(ctx); @@ -865,7 +866,7 @@ static inline int send_sf(struct isotp_send_ctx *ctx) data = get_data_ctx(ctx); pull_data_ctx(ctx, len); - if (ctx->tx_addr.use_ext_addr) { + if ((ctx->tx_addr.flags & ISOTP_MSG_EXT_ADDR) != 0) { frame.data[index++] = ctx->tx_addr.ext_addr; } @@ -895,7 +896,7 @@ static inline int send_sf(struct isotp_send_ctx *ctx) static inline int send_ff(struct isotp_send_ctx *ctx) { struct can_frame frame = { - .flags = ctx->tx_addr.ide != 0 ? CAN_FRAME_IDE : 0, + .flags = (ctx->tx_addr.flags & ISOTP_MSG_IDE) != 0 ? CAN_FRAME_IDE : 0, .id = ctx->tx_addr.ext_id, .dlc = ISOTP_CAN_DL }; @@ -904,7 +905,7 @@ static inline int send_ff(struct isotp_send_ctx *ctx) int ret; const uint8_t *data; - if (ctx->tx_addr.use_ext_addr) { + if ((ctx->tx_addr.flags & ISOTP_MSG_EXT_ADDR) != 0) { frame.data[index++] = ctx->tx_addr.ext_addr; } @@ -936,7 +937,7 @@ static inline int send_ff(struct isotp_send_ctx *ctx) static inline int send_cf(struct isotp_send_ctx *ctx) { struct can_frame frame = { - .flags = ctx->tx_addr.ide != 0 ? CAN_FRAME_IDE : 0, + .flags = (ctx->tx_addr.flags & ISOTP_MSG_IDE) != 0 ? CAN_FRAME_IDE : 0, .id = ctx->tx_addr.ext_id, }; int index = 0; @@ -945,7 +946,7 @@ static inline int send_cf(struct isotp_send_ctx *ctx) int rem_len; const uint8_t *data; - if (ctx->tx_addr.use_ext_addr) { + if ((ctx->tx_addr.flags & ISOTP_MSG_EXT_ADDR) != 0) { frame.data[index++] = ctx->tx_addr.ext_addr; } @@ -1116,7 +1117,8 @@ static void send_work_handler(struct k_work *item) static inline int attach_fc_filter(struct isotp_send_ctx *ctx) { struct can_filter filter = { - .flags = CAN_FILTER_DATA | ((ctx->rx_addr.ide != 0) ? CAN_FILTER_IDE : 0), + .flags = CAN_FILTER_DATA | + ((ctx->rx_addr.flags & ISOTP_MSG_IDE) != 0 ? CAN_FILTER_IDE : 0), .id = ctx->rx_addr.ext_id, .mask = CAN_EXT_ID_MASK }; @@ -1164,7 +1166,7 @@ static int send(struct isotp_send_ctx *ctx, const struct device *can_dev, len = get_ctx_data_length(ctx); LOG_DBG("Send %zu bytes to addr 0x%x and listen on 0x%x", len, ctx->tx_addr.ext_id, ctx->rx_addr.ext_id); - if (len > ISOTP_CAN_DL - (tx_addr->use_ext_addr ? 2 : 1)) { + if (len > ISOTP_CAN_DL - ((ctx->tx_addr.flags & ISOTP_MSG_EXT_ADDR) != 0 ? 2 : 1)) { ret = attach_fc_filter(ctx); if (ret) { LOG_ERR("Can't attach fc filter: %d", ret); diff --git a/tests/subsys/canbus/isotp/conformance/src/main.c b/tests/subsys/canbus/isotp/conformance/src/main.c index 0993ef52d7d..ea55fbb7c52 100644 --- a/tests/subsys/canbus/isotp/conformance/src/main.c +++ b/tests/subsys/canbus/isotp/conformance/src/main.c @@ -72,43 +72,34 @@ const struct isotp_fc_opts fc_opts_single = { .bs = 0, .stmin = 0 }; + const struct isotp_msg_id rx_addr = { .std_id = 0x10, - .ide = 0, - .use_ext_addr = 0 }; const struct isotp_msg_id tx_addr = { .std_id = 0x11, - .ide = 0, - .use_ext_addr = 0 }; const struct isotp_msg_id rx_addr_ext = { .std_id = 0x10, - .ide = 0, - .use_ext_addr = 1, - .ext_addr = EXT_ADDR + .ext_addr = EXT_ADDR, + .flags = ISOTP_MSG_EXT_ADDR, }; const struct isotp_msg_id tx_addr_ext = { .std_id = 0x11, - .ide = 0, - .use_ext_addr = 1, - .ext_addr = EXT_ADDR + .ext_addr = EXT_ADDR, + .flags = ISOTP_MSG_EXT_ADDR, }; const struct isotp_msg_id rx_addr_fixed = { .ext_id = 0x18DA0201, - .ide = 1, - .use_ext_addr = 0, - .use_fixed_addr = 1 + .flags = ISOTP_MSG_FIXED_ADDR | ISOTP_MSG_IDE, }; const struct isotp_msg_id tx_addr_fixed = { .ext_id = 0x18DA0102, - .ide = 1, - .use_ext_addr = 0, - .use_fixed_addr = 1 + .flags = ISOTP_MSG_FIXED_ADDR | ISOTP_MSG_IDE, }; const struct device *const can_dev = DEVICE_DT_GET(DT_CHOSEN(zephyr_canbus)); diff --git a/tests/subsys/canbus/isotp/implementation/src/main.c b/tests/subsys/canbus/isotp/implementation/src/main.c index 1c597f64ec7..bbc2cadb6ce 100644 --- a/tests/subsys/canbus/isotp/implementation/src/main.c +++ b/tests/subsys/canbus/isotp/implementation/src/main.c @@ -35,15 +35,12 @@ const struct isotp_fc_opts fc_opts_single = { .bs = 0, .stmin = 1 }; + const struct isotp_msg_id rx_addr = { .std_id = 0x10, - .ide = 0, - .use_ext_addr = 0 }; const struct isotp_msg_id tx_addr = { .std_id = 0x11, - .ide = 0, - .use_ext_addr = 0 }; struct isotp_recv_ctx recv_ctx;