Skip to content

Handle failed slave selection and stale client buffers safely - #183

Open
rupayon123 wants to merge 2 commits into
arduino-libraries:masterfrom
rupayon123:contribution/reject-invalid-slave-20260921
Open

rupayon123 wants to merge 2 commits into
arduino-libraries:masterfrom
rupayon123:contribution/reject-invalid-slave-20260921

Conversation

@rupayon123

@rupayon123 rupayon123 commented Sep 21, 2026 •

Copy link
Copy Markdown

The client ignores modbus_set_slave() failures and can continue a read/write using an invalid or stale destination. This is the error-propagation problem reported in #151.

Check selection before single reads/writes and before preparing buffered requests. Return each API's documented failure value while retaining the backend error. Reselect the buffered destination at send time and retire the attempted transmission if that selection fails. Address validity stays with libmodbus so RTU/TCP-specific limits are not hard-coded in the client.

A host regression compiles the production ModbusClient.cpp against a recording backend. The original client fails the invalid-ID assertion. The fix passes checks for negative/too-large IDs, all read kinds, single writes, buffered writes, no backend requests after selection failure, valid reads, and buffered destination reselection. Clang C++11 builds with -Wall -Wextra -Werror and address/undefined-behavior sanitizers pass.

This validates wrapper error handling, not the reported physical Opta crash. The RTU client example also compiles for the locally installed Mega AVR core (16,158 bytes flash, 1,647 bytes globals). The libraries warn that AVR is outside their declared architecture lists; this is an additional compile check, not validation of the supported-board CI matrix or physical device behavior. Prepared with AI assistance; source diff and executed regression results reviewed.

A separate buffer-lifetime reproduction found that a failed smaller read leaves the previous larger read count active, so read() can index beyond the resized buffer. end() likewise leaves readable state after freeing the buffer. Invalidate old read/write views before resizing and on shutdown, and retain the old allocation when realloc fails. The extended sanitizer regression covers a failed shrinking read, forced allocation failures, cleanup, and safe post-end reads. This is an additional fix to the same client; it is not a claim of physical-device validation.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@rupayon123 rupayon123 changed the title Stop Modbus requests when slave selection fails Handle failed slave selection and stale client buffers safely Sep 21, 2026
@github-actions

Copy link
Copy Markdown

Memory usage change @ 4f8dcb4

Board flash % RAM for global variables %
arduino:mbed_nano:nano33ble 🔺 0 - +64 0.0 - +0.01 0 - 0 0.0 - 0.0
arduino:mbed_opta:opta 🔺 0 - +64 0.0 - 0.0 0 - 0 0.0 - 0.0
arduino:mbed_portenta:envie_m7 N/A N/A N/A N/A
arduino:megaavr:uno2018:mode=off ❔ -48 - +188 -0.1 - +0.39 0 - 0 0.0 - 0.0
arduino:samd:mkrwifi1010 🔺 0 - +48 0.0 - +0.02 0 - 0 0.0 - 0.0
Click for full report table
Board examples/RTU/ModbusRTUClientKitchenSink
flash
% examples/RTU/ModbusRTUClientKitchenSink
RAM for global variables
% examples/RTU/ModbusRTUClientMD02TemperatureHumiditySensor
flash
% examples/RTU/ModbusRTUClientMD02TemperatureHumiditySensor
RAM for global variables
% examples/RTU/ModbusRTUClientParameters
flash
% examples/RTU/ModbusRTUClientParameters
RAM for global variables
% examples/RTU/ModbusRTUClientToggle
flash
% examples/RTU/ModbusRTUClientToggle
RAM for global variables
% examples/RTU/ModbusRTUServerKitchenSink
flash
% examples/RTU/ModbusRTUServerKitchenSink
RAM for global variables
% examples/RTU/ModbusRTUServerLED
flash
% examples/RTU/ModbusRTUServerLED
RAM for global variables
% examples/RTU/ModbusRTUTemperatureSensor
flash
% examples/RTU/ModbusRTUTemperatureSensor
RAM for global variables
% examples/TCP/EthernetModbusClientToggle
flash
% examples/TCP/EthernetModbusClientToggle
RAM for global variables
% examples/TCP/EthernetModbusServerLED
flash
% examples/TCP/EthernetModbusServerLED
RAM for global variables
% examples/TCP/WiFiModbusClientToggle
flash
% examples/TCP/WiFiModbusClientToggle
RAM for global variables
% examples/TCP/WiFiModbusServerLED
flash
% examples/TCP/WiFiModbusServerLED
RAM for global variables
%
arduino:mbed_nano:nano33ble 64 0.01 0 0.0 64 0.01 0 0.0 0 0.0 0 0.0 64 0.01 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0
arduino:mbed_opta:opta 64 0.0 0 0.0 64 0.0 0 0.0 64 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 64 0.0 0 0.0 0 0.0 0 0.0
arduino:mbed_portenta:envie_m7 N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A
arduino:megaavr:uno2018:mode=off 188 0.39 0 0.0 -48 -0.1 0 0.0 50 0.1 0 0.0 50 0.1 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 58 0.12 0 0.0 40 0.08 0 0.0 0 0.0 0 0.0 40 0.08 0 0.0 0 0.0 0 0.0
arduino:samd:mkrwifi1010 48 0.02 0 0.0 16 0.01 0 0.0 24 0.01 0 0.0 16 0.01 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 16 0.01 0 0.0 24 0.01 0 0.0 0 0.0 0 0.0 24 0.01 0 0.0 0 0.0 0 0.0
Click for full report CSV
Board,examples/RTU/ModbusRTUClientKitchenSink<br>flash,%,examples/RTU/ModbusRTUClientKitchenSink<br>RAM for global variables,%,examples/RTU/ModbusRTUClientMD02TemperatureHumiditySensor<br>flash,%,examples/RTU/ModbusRTUClientMD02TemperatureHumiditySensor<br>RAM for global variables,%,examples/RTU/ModbusRTUClientParameters<br>flash,%,examples/RTU/ModbusRTUClientParameters<br>RAM for global variables,%,examples/RTU/ModbusRTUClientToggle<br>flash,%,examples/RTU/ModbusRTUClientToggle<br>RAM for global variables,%,examples/RTU/ModbusRTUServerKitchenSink<br>flash,%,examples/RTU/ModbusRTUServerKitchenSink<br>RAM for global variables,%,examples/RTU/ModbusRTUServerLED<br>flash,%,examples/RTU/ModbusRTUServerLED<br>RAM for global variables,%,examples/RTU/ModbusRTUTemperatureSensor<br>flash,%,examples/RTU/ModbusRTUTemperatureSensor<br>RAM for global variables,%,examples/TCP/EthernetModbusClientToggle<br>flash,%,examples/TCP/EthernetModbusClientToggle<br>RAM for global variables,%,examples/TCP/EthernetModbusServerLED<br>flash,%,examples/TCP/EthernetModbusServerLED<br>RAM for global variables,%,examples/TCP/WiFiModbusClientToggle<br>flash,%,examples/TCP/WiFiModbusClientToggle<br>RAM for global variables,%,examples/TCP/WiFiModbusServerLED<br>flash,%,examples/TCP/WiFiModbusServerLED<br>RAM for global variables,%
arduino:mbed_nano:nano33ble,64,0.01,0,0.0,64,0.01,0,0.0,0,0.0,0,0.0,64,0.01,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0
arduino:mbed_opta:opta,64,0.0,0,0.0,64,0.0,0,0.0,64,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,64,0.0,0,0.0,0,0.0,0,0.0
arduino:mbed_portenta:envie_m7,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,N/A,,,,,,,,
arduino:megaavr:uno2018:mode=off,188,0.39,0,0.0,-48,-0.1,0,0.0,50,0.1,0,0.0,50,0.1,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,58,0.12,0,0.0,40,0.08,0,0.0,0,0.0,0,0.0,40,0.08,0,0.0,0,0.0,0,0.0
arduino:samd:mkrwifi1010,48,0.02,0,0.0,16,0.01,0,0.0,24,0.01,0,0.0,16,0.01,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,16,0.01,0,0.0,24,0.01,0,0.0,0,0.0,0,0.0,24,0.01,0,0.0,0,0.0,0,0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants