fix(0x11): send ECUReset response before executing reset#78
Conversation
uds_internal_handle_ecu_reset invoked fn_reset before transmitting the 0x51 positive response. A fn_reset that reboots the MCU synchronously (e.g. NVIC_SystemReset) never returned, so the response was lost and the tester saw a timeout instead of confirmation. Send the positive response (or honour suppressPosRsp) first, then invoke fn_reset, per ISO 14229-1:2013 9.3.2.2. If the response cannot be handed to the transport, skip the reset so the tester is not left without a confirmation. Document the uds_reset_fn contract: the hook must defer the physical reset until transmit-complete for async/secured/multi-frame transports, since the library only guarantees ordering, not bus drain. Adds regression tests asserting the response is transmitted strictly before the reset hook fires, and that no reset occurs when the send fails. Closes #76
…ECUReset The previous reorder sent the 0x51 before fn_reset for a plain ECUReset, but when 0x11 is the inner request of a SecuredDataTransmission (0x84) uds_send_response only captures the inner 0x51 — the response the tester actually receives is the outer secured frame, emitted later by the 0x84 handler. So a synchronous fn_reset still fired before that outer frame left the wire. Add a deferred-reset latch (ctx->reset_pending / reset_pending_type): the 0x11 handler queues the reset and runs it immediately for an unsecured or suppressed request, but leaves it pending when the response was captured. The 0x84 handler then fires it after the outer secured response is sent, and cancels it if securing fails. The ROE (0x86) capture path clears the latch so an event-driven inner dispatch can never trigger a reset. Document the now-stronger guarantee on uds_reset_fn and add a regression test asserting the reset fires strictly after the outer secured frame. Refs #76
Follow-up: addressed the self-review items1. Secured/async path was the real gap (not just docs). The reorder alone fixed the plain case, but for a 0x11 wrapped in
So a synchronous-reboot 2. Skip-reset-on-send-failure — settled, not hand-waved. Kept, on principle: ISO 14229-1 makes "response sent" a precondition for the reset, so if the response can't be delivered (TP send fails, or securing fails) the reset is skipped rather than leaving the tester desynchronised without a confirmation. Documented at both the plain and secured failure sites. 3. Stronger test. Added Remaining honest caveat, now documented on Full suite: 59/59. CI gate (cppcheck + clang-format 18) clean in the |
|
Superseded. The basic send-before-reset ordering landed via #82 (a simpler variant), so this PR's reorder is now redundant and conflicts with develop. The additional hardening here — secured (0x84) reset deferral + skip-reset-on-send-failure — has been reworked on top of the merged handler in #85. Closing in favour of #85. |
Problem (issue #76)
The reporter is correct.
uds_internal_handle_ecu_reset(src/services/uds_service_maintenance.c) calledfn_resetbefore transmitting the0x51positive response:ISO 14229-1:2013 §9.3.2.2: "The ECUReset positive response message (if required) shall be sent before the reset is executed in the server(s)."
The natural integrator
fn_reset—NVIC_SystemReset()— never returns, so the response was lost and the tester saw a timeout. The library shipped an example (mock_reset) that only prints, which hid the bug, and the typedef gave no guidance that the hook had to defer.Fix
uds_send_response(which already honourssuppressPosRspinternally — sends nothing when set) before invokingfn_reset.uds_reset_fncontract: the library guarantees ordering relative touds_tp_send_fn, but not that the frame has drained from the bus, nor that a deferred/secured (0x84) or multi-frame response has completed. A hook that tears down the MCU must defer the physical reset until transmit-complete.Known nuance (documented, not silently ignored)
For a 0x84-wrapped or async transport, "on the wire before reset" can't be guaranteed synchronously by the library — that's exactly why the doc requires
fn_resetto defer the physical reset until TX-complete. The reorder fixes the synchronous-reboot footgun; the doc contract covers the rest.Tests
Added two regression guards to
tests/unit/test_service_11.c:test_ecu_reset_response_precedes_reset— a sequencingfn_tp_sendproves the transmit happens strictly before the reset hook fires.test_ecu_reset_no_reset_when_send_fails— a failingfn_tp_sendconfirmsfn_resetdoes not run.Both fail on the pre-fix code and pass with the fix. Full suite: 59/59 pass. CI gate (cppcheck + clang-format 18) verified clean in the
udslib-test-envDocker image.Closes #76