Wire: improve code, add safety checks, add basic logging by JZimnol · Pull Request #171 · zephyrproject-rtos/ArduinoCore-zephyr

JZimnol · 2026-03-11T14:04:37Z

This PR fixes and changes a few things related to the wire port of the Arduino library. All changes are done in separate commits for readability. The most important functions changed are the arduino::ZephyrI2C::requestFrom() and arduino::ZephyrI2C::read().

Additionally, the ARDUINO_API log module/levels have been added enabling an user to control the log level of logs in this repository. As an example, printk() calls have been switched with the LOG_* API.

soburi · 2026-03-11T23:41:27Z

We are considering integrating this part with Arduino, so could you please wait a little while?

Mierunski · 2026-03-12T10:14:07Z

We are using this repository as our intent is to use Arduino libraries within Zephyr application.
How we understand is that https://github.com/arduino/ArduinoCore-zephyr provides flow for using Zephyr functionalities from Arduino application, we did not manage to integrate it with existing Zephyr application (correct me if I'm wrong)

Therefore we would like to have it in this repository

soburi · 2026-03-12T10:34:48Z

We are using this repository as our intent is to use Arduino libraries within Zephyr application. How we understand is that https://github.com/arduino/ArduinoCore-zephyr provides flow for using Zephyr functionalities from Arduino application, we did not manage to integrate it with existing Zephyr application (correct me if I'm wrong)

Therefore we would like to have it in this repository

Yes, we are aware of the issue.
The biggest challenge is that the ArduinoCore-API is LGPL2-licensed and incompatible with Zephyr, which needs to be resolved.
We are considering reimplementing it in Rust, but we're still in the early stages.

DhruvaG2000 · 2026-03-12T10:52:05Z

We are considering integrating this part with Arduino, so could you please wait a little while?

@soburi please could you elaborate on what you mean by this part? Are you saying these fixes are already part of the Arduino fork or you're going to send a PR with these fixes there first?
Just trying to understand

DhruvaG2000 · 2026-03-15T10:18:58Z

-		return (int)buf[0];
+
+  if(!ring_buf_get(&rxRingBuffer.rb, buf, 1)) {
+    printk("\n\nERR: buff empty\n\n\n");


oo many unnecessary \n's .
Just keep it as printk("ERR: buff empty");

Also, elaborate more on the fix itself in the commit "Wire: fix the behavior of the arduino::ZephyrI2C::read()"

DhruvaG2000 · 2026-03-22T12:24:01Z

+  /* Use stack-allocated buffer for reading */
+  uint8_t readbuff[sizeof(rxRingBuffer.buffer)];
+  if (len > sizeof(readbuff)) {
+    printk("\n\nERR: requested length (%zu) exceeds buffer size (%zu) \n\n\n",


Again, too many unnecessary \n's ...

DhruvaG2000 · 2026-03-22T12:32:14Z

  /* Flush the receive buffer so another read() call returns the correct data */
  flush();
-  ret = ring_buf_put(&rxRingBuffer.rb, rxRingBuffer.buffer, len);
+  ret = ring_buf_put(&rxRingBuffer.rb, readbuff, len);


Can you elaborate more on the reason in the commit "Wire: don't use rxRingBuffer.buffer directly for i2c_read() calls"

DhruvaG2000 · 2026-03-22T12:33:01Z

-  } 
+  if (!ring_buf_peek(&rxRingBuffer.rb, buf, 1)){
+    // No data available
+    return -1;


The commit "Wire: fix the behavior of the arduino::ZephyrI2C::peek()" could also use some more explanation in it's body

DhruvaG2000 · 2026-03-22T12:42:50Z


  if(!ring_buf_get(&rxRingBuffer.rb, buf, 1)) {
-    printk("\n\nERR: buff empty\n\n\n");
+    LOG_ERR("buffer is empty");


Ah in the last commit you're changing everything to LOG_ERR. Fine, then you need to drop the printk changes from all your earlier commits because then it's just noise.

@DhruvaG2000 would you like me to squash all those commits into one with a proper name and description then? In the first commits I've been doing some fixes only with the printks being unchanged, that's why it might look misleading. I can squash everything if you want.

@JZimnol no, don't squash. I like how you've partitioned all the fixes. I would just like to see an explanation for each in their own commits, and also those commits need to contain only the fix, you can just remove the printk's from those places, or else just use LOG_ERR in those commits itself.

@DhruvaG2000 I've added required changes + descriptions to some of the commits.

Implemented flush() function to clear the rx buffer. The buffer needs to be flushed so the next read() call returns the proper data. Signed-off-by: Jakub Zimnol <zimnol.jakub@gmail.com>

Removed unecessary ring buffer peek. Additionally added negative value as a return value in case of an error. Signed-off-by: Jakub Zimnol <zimnol.jakub@gmail.com>

Removed a bug from requestFrom() function that caused ring buffer corruption by using rxRingBuffer.buffer directly in i2c_read() calls. Signed-off-by: Jakub Zimnol <zimnol.jakub@gmail.com>

Added a negative return value to peek() in case there's no data to read. Signed-off-by: Jakub Zimnol <zimnol.jakub@gmail.com>

Signed-off-by: Jakub Zimnol <zimnol.jakub@gmail.com>

DhruvaG2000

@soburi are we good to merge this?

soburi · 2026-03-30T03:55:11Z

@soburi are we good to merge this?

Sorry for my late response.

@soburi please could you elaborate on what you mean by this part? Are you saying these fixes are already part of the Arduino fork or you're going to send a PR with these fixes there first?
Just trying to understand

It is fixing the data structure, so it would be better to standardize it in the future, but merging this PR is fine for now.

JZimnol requested review from DhruvaG2000, beriberikix, soburi and szczys as code owners March 11, 2026 14:04

DhruvaG2000 requested changes Mar 22, 2026

View reviewed changes

JZimnol added 6 commits March 25, 2026 11:29

Wire: reset ring buffer during requestFrom() call

147d04f

Implemented flush() function to clear the rx buffer. The buffer needs to be flushed so the next read() call returns the proper data. Signed-off-by: Jakub Zimnol <zimnol.jakub@gmail.com>

Wire: fix the behavior of the arduino::ZephyrI2C::read()

6cf79a4

Removed unecessary ring buffer peek. Additionally added negative value as a return value in case of an error. Signed-off-by: Jakub Zimnol <zimnol.jakub@gmail.com>

Wire: don't use rxRingBuffer.buffer directly for i2c_read() calls

9ebf8ba

Removed a bug from requestFrom() function that caused ring buffer corruption by using rxRingBuffer.buffer directly in i2c_read() calls. Signed-off-by: Jakub Zimnol <zimnol.jakub@gmail.com>

Wire: fix the behavior of the arduino::ZephyrI2C::peek()

939a22d

Added a negative return value to peek() in case there's no data to read. Signed-off-by: Jakub Zimnol <zimnol.jakub@gmail.com>

Wire: avoid buffer overflow in arduino::ZephyrI2C::write(uint8_t)

bba50c7

Signed-off-by: Jakub Zimnol <zimnol.jakub@gmail.com>

Wire: add logging module, add basic error logs

80c15be

Signed-off-by: Jakub Zimnol <zimnol.jakub@gmail.com>

JZimnol force-pushed the wire_lib_improvements branch from 73880fb to 80c15be Compare March 25, 2026 11:22

JZimnol requested a review from DhruvaG2000 March 25, 2026 13:14

DhruvaG2000 approved these changes Mar 29, 2026

View reviewed changes

soburi approved these changes Mar 30, 2026

View reviewed changes

DhruvaG2000 merged commit a2822be into zephyrproject-rtos:next Mar 30, 2026
3 checks passed

Conversation

JZimnol commented Mar 11, 2026

Uh oh!

soburi commented Mar 11, 2026

Uh oh!

Mierunski commented Mar 12, 2026

Uh oh!

soburi commented Mar 12, 2026

Uh oh!

DhruvaG2000 commented Mar 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DhruvaG2000 left a comment

Choose a reason for hiding this comment

Uh oh!

soburi commented Mar 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants