Skip to content

Commit 8577d32

Browse files
linux-wire: fix lifecycle/error handling, preserve TwoWire config, enforce flush failures, add tests
1 parent 2ac309b commit 8577d32

11 files changed

Lines changed: 349 additions & 43 deletions

File tree

.codex

Whitespace-only changes.

docs/api.md

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,17 @@ typedef struct
1212
int fd;
1313
char device_path[LINUX_WIRE_DEVICE_PATH_MAX];
1414
uint32_t timeout_us;
15+
int log_errors;
1516
} lw_i2c_bus;
1617
```
1718

1819
### Bus Management
1920

2021
| Function | Description |
2122
| ----------------------------------------------------------- | -------------------------------------------------------------------------------------------------- |
22-
| `int lw_open_bus(lw_i2c_bus *bus, const char *path);` | Opens `/dev/i2c-X` and populates the handle. Returns `0` on success, `-1` on error (sets `errno`). |
23+
| `int lw_open_bus(lw_i2c_bus *bus, const char *path);` | Opens `/dev/i2c-X` and populates the handle. Returns `0` on success, `-1` on error (sets `errno`) and resets the handle to a closed state on failure. |
2324
| `void lw_close_bus(lw_i2c_bus *bus);` | Closes the file descriptor if open. Safe to call multiple times. |
24-
| `int lw_set_slave(lw_i2c_bus *bus, uint8_t addr);` | Issues `I2C_SLAVE` ioctl to select the target address. |
25+
| `int lw_set_slave(lw_i2c_bus *bus, uint8_t addr);` | Issues `I2C_SLAVE` ioctl to select the target address. Rejects values above `0x7F` with `EINVAL`. |
2526
| `int lw_set_timeout(lw_i2c_bus *bus, uint32_t timeout_us);` | Stores a timeout hint (currently informational). |
2627

2728
### Simple Read/Write
@@ -32,7 +33,7 @@ typedef struct
3233
| `ssize_t lw_read(lw_i2c_bus *bus, uint8_t *data, size_t len);` | Reads up to `len` bytes via `read(2)`. |
3334
| `void lw_set_error_logging(lw_i2c_bus *bus, int enable);` | Enable (`enable != 0`) or suppress (`enable == 0`) `perror` logging for that bus handle. |
3435

35-
Both functions return the number of bytes processed or `-1` on error (`errno` set).
36+
Both functions return the number of bytes processed or `-1` on error (`errno` set). Closed handles report `EBADF`; malformed arguments report `EINVAL`.
3637

3738
### Combined Transactions
3839

@@ -41,7 +42,7 @@ Both functions return the number of bytes processed or `-1` on error (`errno` se
4142
| `ssize_t lw_ioctl_read(lw_i2c_bus *bus, uint16_t addr, const uint8_t *iaddr, size_t iaddr_len, uint8_t *data, size_t len, uint16_t flags);` | Issues an `I2C_RDWR` ioctl with an optional internal register write followed by a read (repeated-start behavior). |
4243
| `ssize_t lw_ioctl_write(lw_i2c_bus *bus, uint16_t addr, const uint8_t *iaddr, size_t iaddr_len, const uint8_t *data, size_t len, uint16_t flags);` | Builds a single write message combining an optional internal address and payload. |
4344

44-
The helper validates inputs (non-null buffers, length ≤ 4096, etc.) before calling into the kernel.
45+
The helpers validate inputs (non-null buffers, length ≤ 4096, etc.) before calling into the kernel. Closed handles report `EBADF`; malformed arguments report `EINVAL`.
4546

4647
---
4748

@@ -53,39 +54,43 @@ The helper validates inputs (non-null buffers, length ≤ 4096, etc.) before cal
5354

5455
| Method | Description |
5556
| ------------------------------------------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------- |
56-
| `void begin(const char *device = "/dev/i2c-1");` | Opens the specified I²C device. Calling `begin()` again reopens the bus. |
57+
| `void begin(const char *device = "/dev/i2c-1");` | Opens the specified I²C device. Calling `begin()` again reopens the bus and re-applies stored timeout/logging preferences. |
5758
| `void begin(uint8_t); void begin(int);` | Provided for Arduino compatibility; they’re no-ops in Linux master mode. |
5859
| `void end();` | Closes the bus and clears buffers. |
5960
| `void setClock(uint32_t frequency);` | Currently a no-op (bus speed is controlled by the kernel). |
60-
| `void setWireTimeout(uint32_t timeout_us = 25000, bool reset_with_timeout = false);` | Stores a timeout threshold; when the underlying I/O reports `ETIMEDOUT`, `getWireTimeoutFlag()` becomes true and (optionally) the bus is reopened. |
61+
| `void setWireTimeout(uint32_t timeout_us = 25000, bool reset_with_timeout = false);` | Stores a timeout threshold; when the underlying I/O reports `ETIMEDOUT`, `getWireTimeoutFlag()` becomes true and (optionally) the bus is reopened with the same timeout hint still applied. |
6162
| `bool getWireTimeoutFlag() const;` / `void clearWireTimeoutFlag();` | Query/reset the timeout flag. |
62-
| `void setErrorLogging(bool enable);` | Toggle low-level `perror` logging (handy when probing addresses that are expected to NACK). |
63+
| `void setErrorLogging(bool enable);` | Toggle low-level `perror` logging (handy when probing addresses that are expected to NACK). The preference survives reopen operations. |
6364

6465
### Master Transmit
6566

6667
| Method | Description |
6768
| ------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
6869
| `void beginTransmission(uint8_t address);` | Starts buffering data for the given device. |
6970
| `void beginTransmission(int address);` | Overload that forwards to the `uint8_t` version. |
70-
| `uint8_t endTransmission(uint8_t sendStop = 1);` | Writes the buffered bytes. Return codes match Arduino: `0` success, `1` buffer overflow, `4` other error. Passing `0` for `sendStop` defers the actual write until the next `requestFrom` (repeated-start semantics). |
71+
| `uint8_t endTransmission(uint8_t sendStop = 1);` | Writes the buffered bytes. Return codes match Arduino: `0` success, `1` buffer overflow, `4` other error. Passing `0` for `sendStop` defers the actual write until the next `requestFrom` (repeated-start semantics). If an older deferred write must be auto-flushed first and that flush fails, this call returns `4`. |
7172
| `size_t write(uint8_t data);` / `size_t write(const uint8_t *data, size_t len);` / `size_t write(const char *str);` | Append data to the TX buffer (up to 32 bytes). |
7273

7374
### Master Receive
7475

7576
| Method | Description |
7677
| ------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------- |
77-
| `uint8_t requestFrom(uint8_t address, uint8_t quantity, uint8_t sendStop = 1);` | Reads up to `quantity` bytes. If prior `endTransmission(false)` buffered data for the same address, it performs a combined `I2C_RDWR` transaction. |
78+
| `uint8_t requestFrom(uint8_t address, uint8_t quantity, uint8_t sendStop = 1);` | Reads up to `quantity` bytes. If prior `endTransmission(false)` buffered data for the same address, it performs a combined `I2C_RDWR` transaction. If a deferred write must be auto-flushed first and that flush fails, the read is aborted and `0` is returned. |
7879
| `uint8_t requestFrom(uint8_t address, uint8_t quantity, uint32_t iaddress, uint8_t isize, uint8_t sendStop);` | Arduino-style register helper; `isize` is clamped to 4. |
7980
| `uint8_t requestFrom(int address, int quantity);` / `uint8_t requestFrom(int address, int quantity, int sendStop);` | Compatibility overloads. |
8081
| `int available() const; int read(); int peek(); void flush();` | Buffer inspection helpers matching Arduino semantics. |
8182

8283
### Repeated-start semantics
8384

84-
- `endTransmission(false)` leaves the TX buffer intact for the next `requestFrom` to the same address. If you later target a different address or call `beginTransmission` without a read in between, linux-wire automatically flushes the pending data with a STOP so nothing is silently dropped.
85+
- `endTransmission(false)` leaves the TX buffer intact for the next `requestFrom` to the same address.
86+
- If you later target a different address or call `beginTransmission` without a read in between, linux-wire automatically flushes the pending data with a STOP.
87+
- If that automatic flush fails, the follow-on operation fails instead of silently proceeding.
8588

8689
### Timeout behavior
8790

88-
- When `lw_read`/`lw_ioctl_read` reports `ETIMEDOUT`, `wireTimeoutFlag_` is set. If `reset_with_timeout` was true when `setWireTimeout` was called, the bus descriptor is closed and reopened automatically.
91+
- When `lw_read`/`lw_ioctl_read` reports `ETIMEDOUT`, `wireTimeoutFlag_` is set.
92+
- If `reset_with_timeout` was true when `setWireTimeout` was called, the bus descriptor is closed and reopened automatically.
93+
- Stored timeout and error-logging preferences are re-applied after that reopen.
8994

9095
---
9196

@@ -97,4 +102,4 @@ See the `examples/` directory for concrete flows:
97102
- `master_writer`: simple register write.
98103
- `master_reader`: demonstrates `endTransmission(false)` + `requestFrom` repeated-start read.
99104

100-
For mock-based tests of buffer management and timeout logic, inspect `tests/test_wire.cpp`.
105+
For mock-based tests of buffer management, deferred-write failure handling, and timeout logic, inspect `tests/test_wire.cpp`.

docs/index.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,12 +90,12 @@ int main() {
9090
Important Arduino-parity notes:
9191

9292
- `requestFrom(address, quantity, iaddress, isize, sendStop)` exists and clamps `isize` to 4 bytes, mirroring the upstream Wire overload.
93-
- Calling `endTransmission(false)` leaves the TX buffer intact for the next `requestFrom` to the same address. If no read follows, the buffer is flushed with a STOP before the next transmission to avoid silently dropping data.
94-
- `setWireTimeout(timeout_us, reset_on_timeout)` sets a flag when Linux reports `ETIMEDOUT`. If `reset_on_timeout` is true, the bus handle is closed and reopened automatically (matching the AVR `twi` reset behavior).
93+
- Calling `endTransmission(false)` leaves the TX buffer intact for the next `requestFrom` to the same address. If no read follows, the buffer is flushed with a STOP before the next operation; if that automatic flush fails, the follow-on operation fails instead of silently continuing.
94+
- `setWireTimeout(timeout_us, reset_on_timeout)` sets a flag when Linux reports `ETIMEDOUT`. If `reset_on_timeout` is true, the bus handle is closed and reopened automatically (matching the AVR `twi` reset behavior), and stored timeout/logging preferences are re-applied.
9595

9696
## Testing
9797

98-
Software-only tests live in `tests/`. They use a mock `lw_*` backend to exercise buffer management, repeated-start logic, and timeout handling without real hardware:
98+
Software-only tests live in `tests/`. They use a mock `lw_*` backend to exercise buffer management, repeated-start logic, deferred-write failure handling, and timeout handling without real hardware:
9999

100100
```sh
101101
ctest --test-dir build --output-on-failure

include/Wire.h

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,8 @@ class TwoWire
6464
* Wire.begin("/dev/i2c-0"); // Uses /dev/i2c-0
6565
*
6666
* Note: If already open, this will close and reopen the bus.
67+
* Stored timeout and error-logging preferences are re-applied
68+
* after every successful open.
6769
*/
6870
void begin(const char *device = "/dev/i2c-1");
6971

@@ -106,6 +108,8 @@ class TwoWire
106108
*
107109
* Note: Timeout enforcement is currently informational only.
108110
* Future versions may implement actual timeout via select/poll.
111+
* The configured timeout hint is preserved across `begin()` and
112+
* timeout-triggered reopen operations.
109113
*/
110114
void setWireTimeout(uint32_t timeout_us = 25000, bool reset_with_timeout = false);
111115

@@ -124,6 +128,9 @@ class TwoWire
124128
/**
125129
* Enable or disable error messages printed by the low-level I2C helpers.
126130
* Useful when deliberately probing addresses that will NACK (e.g., strict scanner).
131+
*
132+
* The chosen setting is preserved across `begin()` and timeout-triggered
133+
* reopen operations.
127134
*/
128135
void setErrorLogging(bool enable);
129136

@@ -159,6 +166,9 @@ class TwoWire
159166
* requestFrom() call, avoiding an intervening STOP condition.
160167
* The actual STOP is still issued by the kernel driver; this
161168
* just controls whether to use combined ioctl transactions.
169+
* If an older deferred write must be auto-flushed first and that
170+
* flush fails, this method returns 4 and does not send the new
171+
* transmission.
162172
*/
163173
uint8_t endTransmission(uint8_t sendStop);
164174
uint8_t endTransmission(void);
@@ -177,6 +187,10 @@ class TwoWire
177187
* After successful call, use available(), read(), and peek() to
178188
* access the received data.
179189
*
190+
* If a deferred write from `endTransmission(false)` must be auto-flushed
191+
* before starting this read and that flush fails, this method returns 0
192+
* and does not start the read.
193+
*
180194
* Examples:
181195
* // Simple read
182196
* Wire.requestFrom(0x40, 2);
@@ -245,6 +259,7 @@ class TwoWire
245259
private:
246260
lw_i2c_bus bus_;
247261
bool bus_open_;
262+
bool errorLoggingEnabled_;
248263

249264
char devicePath_[LINUX_WIRE_DEVICE_PATH_MAX];
250265
uint8_t txAddress_;
@@ -266,6 +281,7 @@ class TwoWire
266281

267282
void resetTxBuffer();
268283
void resetRxBuffer();
284+
void applyBusConfiguration();
269285

270286
uint8_t requestFrom(uint8_t address,
271287
uint8_t quantity,

include/linux_wire.h

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ extern "C"
2727
* device_path - Path used to open the bus (e.g., "/dev/i2c-1")
2828
* timeout_us - Timeout value in microseconds (0 = no timeout)
2929
* Currently informational only; not enforced by implementation
30+
* log_errors - Non-zero enables perror logging for low-level failures
3031
*/
3132
typedef struct
3233
{
@@ -51,7 +52,7 @@ extern "C"
5152
* EACCES - Permission denied (user may need to be in 'i2c' group)
5253
*
5354
* On success, bus->fd contains a valid file descriptor.
54-
* On failure, bus->fd is set to -1.
55+
* On failure, the bus handle is reset to a closed state (`fd == -1`).
5556
*
5657
* Example:
5758
* lw_i2c_bus bus;
@@ -100,9 +101,9 @@ extern "C"
100101
* @return Number of bytes written on success, -1 on error (errno set)
101102
*
102103
* Error conditions:
103-
* EINVAL - Invalid parameters
104-
* EBADF - Bus not open
105-
* ENXIO - No device at selected address (NACK)
104+
* EINVAL - Invalid parameters (NULL pointers, NULL buffer with len > 0)
105+
* EBADF - Bus not open
106+
* ENXIO - No device at selected address (NACK)
106107
* ETIMEDOUT - Communication timeout
107108
*
108109
* Note: The send_stop parameter is accepted for API compatibility but
@@ -125,7 +126,7 @@ extern "C"
125126
* @return Number of bytes read on success, -1 on error (errno set)
126127
*
127128
* Error conditions:
128-
* EINVAL - Invalid parameters
129+
* EINVAL - Invalid parameters (NULL pointers, NULL buffer with len > 0)
129130
* EBADF - Bus not open
130131
* ENXIO - No device at selected address (NACK)
131132
* ETIMEDOUT - Communication timeout
@@ -232,6 +233,9 @@ extern "C"
232233
*
233234
* @param bus Pointer to lw_i2c_bus
234235
* @param enable Non-zero to enable logging, zero to suppress
236+
*
237+
* This preference is also re-applied by the C++ wrapper after automatic
238+
* timeout recovery and fresh `begin()` calls.
235239
*/
236240
void lw_set_error_logging(lw_i2c_bus *bus, int enable);
237241

0 commit comments

Comments
 (0)