Skip to content

Fix for confusing overload of Wire::begin #6984

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.

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

Closed
wants to merge 3 commits into from
Closed

Fix for confusing overload of Wire::begin #6984

wants to merge 3 commits into from

Conversation

bato3
Copy link

@bato3 bato3 commented Jul 14, 2022

Closes #6616

SDA and SCL are defined as uint8_t, and when it is used, library try initialize itself in slave mode

By completing this PR sufficiently, you help us to review this Pull Request quicker and also help improve the quality of Release Notes

Checklist

  1. Please provide specific title of the PR describing the change, including the component name (eg. „Update of Documentation link on Readme.md“)
  2. Please provide related links (eg. Issue which will be closed by this Pull Request)
  3. Please update relevant Documentation if applicable
  4. Please check Contributing guide

This entire section above can be deleted if all items are checked.


Description of Change

Please describe your proposed Pull Request and it's impact.

Tests scenarios

Please describe on what Hardware and Software combinations you have tested this Pull Request and how.

(eg. I have tested my Pull Request on Arduino-esp32 core v2.0.2 with ESP32 and ESP32-S2 Board with this scenario)

Related links

Please provide links to related issue, PRs etc.

(eg. Closes #number of issue)

Fixes #6616

SDA and SCL are defined as `uint8_t`, and when it is used, library try initialize itself in slave mode
@CLAassistant
Copy link

CLAassistant commented Jul 14, 2022

CLA assistant check
All committers have signed the CLA.

@bato3 bato3 changed the title Fox for confusing overload of Wire::begin Fix for confusing overload of Wire::begin Jul 14, 2022
@SuGlider SuGlider self-requested a review July 14, 2022 14:36
@SuGlider SuGlider self-assigned this Jul 14, 2022
@SuGlider SuGlider added this to the 2.0.5 milestone Jul 14, 2022
@SuGlider
Copy link
Collaborator

@bato3

When compilling it I got this message in the console:

In file included from C:\Users\SuGlider\Documents\Arduino\issue_6616_wire_slave_begin_int8\issue_6616_wire_slave_begin_int8.ino:1:
C:\Users\SuGlider\Documents\Arduino\hardware\espressif\esp32\libraries\Wire\src/Wire.h: In function 'void setup()':
C:\Users\SuGlider\Documents\Arduino\hardware\espressif\esp32\libraries\Wire\src/Wire.h:84:10: note: candidate 1: 'bool TwoWire::begin(uint8_t, int, int, uint32_t)'
     bool begin(uint8_t slaveAddr, int sda=-1, int scl=-1, uint32_t frequency=0);
          ^~~~~
C:\Users\SuGlider\Documents\Arduino\hardware\espressif\esp32\libraries\Wire\src/Wire.h:80:17: note: candidate 2: 'bool TwoWire::begin(uint8_t, uint8_t, uint32_t)'
     inline bool begin(uint8_t sda, uint8_t scl, uint32_t frequency=0)
                 ^~~~~

@SuGlider
Copy link
Collaborator

I see other considerations as exposed in #6616 (comment)

Copy link
Collaborator

@SuGlider SuGlider left a comment

Choose a reason for hiding this comment

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

Please detail which Use Cases are covered by this PR.

@SuGlider
Copy link
Collaborator

@bato3 - Please evaluate this other PR #7000 to solve the issue.

@bato3
Copy link
Author

bato3 commented Jul 18, 2022

@SuGlider #7000 has case

Wire.begin(SDA, SCL);

so, it can be OK.
I don't have the skills to test it properly.

@bato3 bato3 closed this Jul 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Confusing overload of Wire::begin
3 participants