esp8266 / Arduino

ESP8266 core for Arduino
GNU Lesser General Public License v2.1
16.06k stars 13.33k forks source link

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

Open tablatronix opened 7 years ago

tablatronix commented 7 years ago

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 commented 7 years ago

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:

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.

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 commented 7 years ago

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.