Skip to content

libraries/Wire: Add support for target mode. #169

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Aug 27, 2025
Merged

libraries/Wire: Add support for target mode. #169

merged 4 commits into from
Aug 27, 2025
+197 −81

Conversation

iabdalkader
Copy link

@iabdalkader iabdalkader commented Aug 5, 2025 •
edited
Loading

Test sketch:

#include <Wire.h>

void onRequest() {
  Wire.write("HelloWorld!");
}

void onReceive(int howMany) {
  while (Wire.available()) {
    char c = Wire.read();
    Serial.print(c);
  }
  
  if (howMany) {
    Serial.println("");
  }
}

void setup() {
  Serial.begin(115200);
  Wire.begin(8);
  Wire.onRequest(onRequest);
  Wire.onReceive(onReceive);
}

void loop() {
  delay(1000);
}

Retested master mode with VL53L1X and target mode with bus pirate:

image

@iabdalkader iabdalkader mentioned this pull request Aug 5, 2025
Closed
@iabdalkader iabdalkader requested review from pennam and facchinm August 5, 2025 13:06
@iabdalkader iabdalkader force-pushed the wire_slave_support branch 3 times, most recently from 9ce1ca5 to e4ed02a Compare August 5, 2025 13:47
pennam
pennam requested changes Aug 26, 2025
View reviewed changes
Copy link

@pennam pennam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iabdalkader I've tested this PR on a Nano 33 BLE sense Rev2 and I2C controller mode is not working anymore.

I've used this example.

With the released core i get this output:

15:12:48.431 -> Pressure = 100.55 kPa
15:12:48.431 -> Temperature = 28.37 C
15:12:48.431 -> 
15:12:49.428 -> Pressure = 100.54 kPa
15:12:49.428 -> Temperature = 28.35 C

using this PR instead i get this:

Failed to initialize pressure sensor!

I think it would be better to split this commit an put the "Fix buffer overflows, bad return values etc.." in a separate commit /PR because i suspect something in those changes has break I2C controller mode.

@iabdalkader
Copy link
Author

I think it would be better to split this commit an put the "Fix buffer overflows, bad return values etc.."

That's not possible, the changes are required for Target mode.

@pennam
Copy link

pennam commented Aug 26, 2025

The issue is now requestFrom is returning 0 on success and it should return the number of bytes returned from the peripheral.

vl5xl1x library works because is not checking the return value but other libraries do it

@iabdalkader
loader: Export missing ring_buf symbols.
a1c4db8 
Used by Wire.

Signed-off-by: iabdalkader <[email protected]>
@iabdalkader
Copy link
Author

iabdalkader commented Aug 26, 2025 •
edited
Loading

it should return the number of bytes returned from the peripheral.

Except that doesn't specify what to return if it fails to read. Also, the return is unsigned so we can't return -1. I fixed it anyway, but now it's not possible to know if it failed or the buffer is just full.

@pennam
Copy link

pennam commented Aug 26, 2025

I guess 0 is the only option we have to point out an error, considering a request of 0 bytes an edge case.

pennam
pennam reviewed Aug 26, 2025
View reviewed changes
pennam
pennam requested changes Aug 27, 2025
View reviewed changes
@iabdalkader
libraries/Wire: Add support for target mode.
f8d044f 
- Add support for I2C target mode.
- Fix buffer overflows, bad return values etc..

Signed-off-by: iabdalkader <[email protected]>
@iabdalkader
variants/arduino_giga_r1_stm32h747xx_m7: Enable I2C target.
af01ce4 
Enable target support for Giga R1.

Signed-off-by: iabdalkader <[email protected]>
@iabdalkader
variants/arduino_portenta_h7_stm32h747xx_m7: Enable I2C target.
a18719b 
Enable target support for Giga R1.

Signed-off-by: iabdalkader <[email protected]>
pennam
pennam approved these changes Aug 27, 2025
View reviewed changes
Copy link

@pennam pennam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I2CSCanner 👍
LPS22HB ReadPressure 👍
I2CMasterReader/I2CSlaveWriter 👍
I2CMasterWriter/I2CSlaveReceiver 👍

@pennam pennam merged commit 6e8af48 into arduino:main Aug 27, 2025
15 checks passed
@iabdalkader iabdalkader deleted the wire_slave_support branch August 27, 2025 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pennam pennam pennam approved these changes

@facchinm facchinm Awaiting requested review from facchinm

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@iabdalkader @pennam