Skip to content
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

Ported from esp8266/Arduino i2c HAL and Wire library #1061

Merged
merged 1 commit into from
Apr 20, 2017

Conversation

avr39-ripe
Copy link
Contributor

Ported from esp8266/Arduino i2c HAL, faster by direct pin manipulation
not via digitalWrite, some additional features of i2c bus implemented
Wire library implement some additional features such setClock.
Looks like being fully compatible with former implementation of both i2c HAL and Wiring lib.
Tested with DS3232RTC_NTP_Setter, LiquidCrystal_44780 samples and my app with MCP23017

Ported from esp8266/Arduino i2c HAL, faster by direct pin manipulation
not via digitalWrite, some additional features of i2c bus implemented
}

TwoWire Wire = TwoWire(I2C_DEFAULT_SCL_PIN, I2C_DEFAULT_SDA_PIN);
void TwoWire::onReceive( void (*function)(int) ){
//user_onReceive = function;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the onReceive callback stuff is commented out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is PORT of the library with as minimum changes as possible. May be we should import it as is, try how it works and then add more useful stuff to it as delegates etc??

};

#if !defined(NO_GLOBAL_INSTANCES) && !defined(NO_GLOBAL_TWOWIRE)
extern TwoWire Wire;
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be better, memory wise, to avoid creating global objects. Therefore I would prefere the global instance to be created only if requested (in the code above it is created by default).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Creating default Wire object is "good?? old Arduino practice" so I think we should not invent wheel and disable this. IMHO.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, also if no-one uses the Wire object, linker will not include it in final binary so no memory impact

public:
TwoWire();
void begin(int scl, int sda);
void pins(int scl, int sda);
Copy link
Contributor

@zgoda zgoda Apr 7, 2017

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, deprecated, but it widely used in our samples and it do not hurts, so IMHO it should be included. Moreover, I changed SCL/SDA definition order from original SDA,SCL to used now in samples//libs/etc SCL/SDA.

Copy link
Contributor

Choose a reason for hiding this comment

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

This will require porting for more hw libs as the argument order is reversed - or writing compat layers on top. Keeping compatibility with Arduino will make drop-in replacement for Wire.h. Hard to say what will be better although I don't mind writing some code to make existing lib working.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we should decide either rewrite current samples/libs to use SDA/SCL pin order or rewrite every next lib with Wire. Even after my change of pin order I think it was mistake. Better change pin order in known list of our samples/libs and our own code and then have compatibility with Arduino. Comments are welcome! Thank you @zgoda to point this issue!

@slaff slaff self-assigned this Apr 20, 2017
@slaff slaff added this to the 3.2.0 milestone Apr 20, 2017
@slaff slaff merged commit 0c2d053 into SmingHub:develop Apr 20, 2017
@slaff slaff mentioned this pull request Apr 26, 2017
16 tasks
slaff pushed a commit to slaff/Sming that referenced this pull request Jul 18, 2017
…Wire.pins parameters to

match the [Arduino order](samples/DS3232RTC_NTP_Setter/app/application.cpp).

Related to SmingHub#1179 and SmingHub#1061.

Warning: this is a backward incompatible change.
slaff added a commit that referenced this pull request Jul 27, 2017
…Wire.pins parameters to (#1193)

match the [Arduino order](samples/DS3232RTC_NTP_Setter/app/application.cpp).

Related to #1179 and #1061.

Warning: this is a backward incompatible change.
@slaff slaff removed the 3 - Review label Sep 29, 2018
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

Successfully merging this pull request may close these issues.

4 participants