feat(isotp): Add thread safety, callbacks, and extended ID support (IEC-370) #563

eternal-echo · 2025-09-05T05:41:32Z

Overview

This PR enhances the esp_isotp component with thread safety, event-driven callbacks, and extended feature support, transforming it from a basic polling implementation to a production-ready ISO-TP communication library.

Key Features

Thread Safety & ISR Support

ISR-safe send functions: esp_isotp_send() and esp_isotp_send_with_id() can be called from interrupt context
Spinlock protection: Core state protected with portMUX locks to prevent race conditions
TX frame buffer pool: Pre-allocated memory pool eliminates stack corruption in async TWAI operations

Event-Driven Callbacks

Asynchronous notifications: Optional rx_callback and tx_callback in configuration
Non-blocking operation: Eliminates need for continuous polling in callback mode

Extended Features

29-bit ID support: Added use_extended_id configuration option
Dynamic TX ID: New esp_isotp_send_with_id() function for runtime ID override
Enhanced error handling: More specific esp_err_t return codes

API Changes

New Configuration Options

esp_isotp_config_t config = {
    .use_extended_id = false,
    .tx_frame_pool_size = 8,
    .rx_callback = my_rx_callback,
    .tx_callback = my_tx_callback,
    .callback_arg = user_data
};

New Functions

esp_err_t esp_isotp_send_with_id(esp_isotp_handle_t handle, uint32_t id, const uint8_t *data, uint32_t size)

Breaking Changes

None. This enhancement maintains full backward compatibility.

Files Modified

src/esp_isotp.c: Core implementation with memory management and thread safety
inc/esp_isotp.h: API enhancements and documentation
README.md: Updated documentation with usage examples

Notes

esp_isotp_poll() must still be called periodically for multi-frame message handling
RX callbacks execute in ISR context - keep processing minimal
TX frame pool prevents memory corruption in async operations

Checklist

Component contains README.md
Component contains idf_component.yml file with url field defined
Component contains unit tests (pytest)

esp_isotp/src/esp_isotp.c

-    // Set user argument for TWAI operations
+    // Pre-allocate ISR-safe receive frame buffer to avoid dynamic allocation in interrupt context.
+    // This buffer is reused for each incoming TWAI frame received in the ISR.
+    memset(&isotp->isr_rx_frame_buffer, 0, sizeof(esp_isotp_frame_t));


esp_isotp/isotp-c/isotp_user.h

esp_isotp/inc/esp_isotp.h

esp_isotp/src/esp_isotp.c

+    ESP_RETURN_ON_FALSE_ISR(tx_frame != NULL, ISOTP_RET_ERROR, TAG, "No available frames in pool");
+
+    // Initialize TWAI frame header and copy payload data into embedded buffer.
+    memset(&tx_frame->frame, 0, sizeof(twai_frame_t));


Copilot

Pull Request Overview

This PR transforms the esp_isotp component from a basic polling implementation to a production-ready ISO-TP library with thread safety, event-driven callbacks, and extended feature support.

Adds thread safety with spinlock protection and ISR-safe send functions
Introduces event-driven callbacks (rx_callback, tx_callback) for asynchronous notifications
Implements 29-bit extended ID support with automatic format detection

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
esp_isotp/src/esp_isotp.c	Core implementation with thread safety, memory management, callback system, and extended ID support
esp_isotp/inc/esp_isotp.h	API enhancements with callback types, updated function signatures, and comprehensive documentation
esp_isotp/README.md	Updated documentation with usage examples, extended ID configuration, and error handling guidance

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

esp_isotp/src/esp_isotp.c

esp_isotp/inc/esp_isotp.h

esp_isotp/src/esp_isotp.c

+    ESP_RETURN_ON_FALSE_ISR(size <= 8, ISOTP_RET_ERROR, TAG, "Invalid TWAI frame size");
+
+    // Copy payload into the embedded buffer to ensure data lifetime during async transmission.
+    memcpy(tx_frame->data_payload, data, size);


CLAassistant · 2025-09-10T05:56:39Z

All committers have signed the CLA.

…d safety features

…ynchronous sending by pre-alloc a list tx_buffers. Now the esp_isotp_send(_with_id) supports interrupt context calls

esp_isotp/inc/esp_isotp.h

esp_isotp/src/esp_isotp.c

suda-morris

LGTM. The using of the link_lock to protect all isotpc API is a little bit overkill to me.

Copilot

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

esp_isotp/src/esp_isotp.c

esp_isotp/inc/esp_isotp.h

suda-morris · 2025-09-15T10:07:38Z

esp_isotp/src/esp_isotp.c

+    tx_frame->frame.header.ide = is_extended_id(arbitration_id);  // Extended (29-bit) vs Standard (11-bit) ID
+
+    // Size validation - TWAI frames are max 8 bytes by protocol
+    ESP_RETURN_ON_FALSE_ISR(size <= 8, ISOTP_RET_ERROR, TAG, "Invalid TWAI frame size");


this check should be moved to the begging of the function. @eternal-echo

eternal-echo marked this pull request as ready for review September 5, 2025 05:42

github-advanced-security bot found potential problems Sep 5, 2025

View reviewed changes

suda-morris requested a review from Copilot September 5, 2025 06:58

This comment was marked as outdated.

Sign in to view