Skip to content

suggestion , do not deprecate wire.pins() #2774

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

Open
tablatronix opened this issue Dec 18, 2016 · 2 comments
Open

suggestion , do not deprecate wire.pins() #2774

tablatronix opened this issue Dec 18, 2016 · 2 comments

Comments

@tablatronix
Copy link
Contributor

I notice wire.pins() is marked deprecated.

Please consider, I did not see an explanation for reasoning, maybe I missed it.

A lot of libraries have a Wire.begin() inside their own begin() or init function.
Then they immediately send initialization codes to lcds or oleds.

Short of modifying libraries, or pushing authors to have to modify their code to include arguments for passing through sda and scl, there is no way other than calling pins() or begin() before the libraries begin() to change the pins. Also to be pedantic begin is now being called twice once for just kludging the pins then inside the library using wire and also makes it confusing if you wanted to toggle pins back and forth so you can use twi on 2 buses.

It is easier to do something like ( granted this is not ideal but if you must use twi twice )

//init
Wire.begin(0,1);
Wire.begin(3,4);
Wire.setSpeed(400000); // must be reset after a begin always !

lcd.begin();

lcd.write(blargh);
Wire.pins(0,1); // dynamic pin reassignment WITHOUT calling twi_init and losing custom clock or stretch coding.
lcd.write(blargh);

❗️ So another issue with calling begin is that it hardcodes clock and stretch in twi_init, so you must keep redoing those. This might be another "bug" that can be improved by using twi_dcount and twi_clockStretchLimit in twi_init instead of hardcoded values, and init those globals instead.

@bperrybap
Copy link

bperrybap commented Dec 22, 2016

I agree that there is lots of library code that exists that calls Wire.begin() and there needs to be solution to allow a sketch to modify the pins and clock of the Wire object that does not require modifying libraries.
And that allows multiple calls to Wire.begin() to not undue desired settings for the Wire object.
i.e. if a sketch wants to run different clock rate, then once configured future Wire.begin() should not undue that setting.

I have a library right now (hd44780) that calls Wire.begin().
I'm also wrestling with how to deal with supporting TwoWire object names other than "Wire".
I'm looking at using templates to help solve the TwoWire object name issue when a different object name other than "Wire" is used, but not all libraries using "Wire" libraries are going to do this.

Not sure about the toggling/altering of pins for using multiple i2c buses for the single Wire object. That one seems a bit odd as normally there are different objects for each bus, but I get the reasoning behind it when using libraries that have hard coded their object name to "Wire" particularly if using multiple libraries on different buses.

For TwoWire objects without toggling pins for multiple buses, here is another approach:
Allow the sketch to create a TwoWire object that overrides the Wire object in Wire.cpp and create a new TwoWire constructor that allows the sketch to set the default values for pins & speed when it declares the Wire object.

It looks like this is all that needs to be done:

  • move the variables default_sda_pin and default_scl_pin inside the TwoWrire class & make them const values.
    (C++ allows global static variables inside classes - they are global across all object instances)

  • create a new new constructor for the TwoWire class that takes two arguments (sda, scl) and use initializers to assign them to the static const default_sda_pin and default_scl_pin variables in the class.
    (constructor initializers can assign values to const members)
    Perhaps another optional parameter to the TwoWire constructor could be added to set the clock speed.

  • make the TwoWire Wire object in Wire.cpp a weak symbol.

The last part of using a weak symbol is the main key to making all this work.
By doing all this, a sketch could declare its own wire object:
TwoWire Wire(sda_pin, scl_pin);
or
TwoWire Wire(sda_pin, scl_pin, wire_speed);

And that Wire object declaration in the sketch would override & replace the one in Wire.cpp
This offers the desired result of being able to set the pins and clock speed without having to use other functions.

i.e. this gives the sketch a way to declare the Wire object itself along with overriding the default pins and speed if it does not want to use the Wire library defaults.
And then any calls to Wire.begin() would simply use the configured defaults.
It would also allow creating additional TwoWire objects with different names with different pins and clock speeds.

These changes are fairly minimal, should be difficult to implement, and I'd be happy to submit a pull request if there is interest in going down this path.

IMO, using constructors to set default parameters in "Wire" objects is better than using functions to set defaults pins and speeds in that typically these are not things that are changed for a given bus/environment and it keeps this type of stuff out of the runtime code and libraries.

All that said this does not solve the issue of wanting to use multiple libraries that are hard coded to use the Wire object on different buses.
That requires the ability to run-time override the pins and clock using functions.

Perhaps the ultimate solution is to do both.

  • preserve the existing run time functions for tricking libraries to use other buses using the single Wire object
  • the ability of the sketch to declare a Wire object(s) with defaults, to override the one in Wire.cpp

Note: other Wire libraries like the one for the PIC32 core, do more like keep track of the Wire.begin() calls inside the object and only the first one does anything.
They also have an end() function that which decrements the "begin" counter to release the i2c h/w and allow reconfiguration on the next begin() should it be necessary.

@tablatronix
Copy link
Contributor Author

And doing what I described is clearly a kludge, I had 2 devices with same ID and needed to run them both right then as was surprised it actually worked well. So I limited my issue to that ( and compiler flags CAN cause issues with deprecated flags for pins() )

I figured any solution involving instancing twi class better for wire would be overly complicated, but you seem to understand and have solutions.

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

No branches or pull requests

2 participants