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

NON-OS SDK support. Tested on v2.0.0. #10

Merged
merged 1 commit into from
Sep 18, 2016
Merged

Conversation

valkuc
Copy link
Contributor

@valkuc valkuc commented Sep 14, 2016

Related discussion - #9

@pasko-zh
Copy link
Owner

Thanks! Will check on the weekend!

// 7 Bit Slave Address; SCL Frequency in Steps of 100 KHz, range: 100 -- 1000 KHz

i2c_slave_address = slave_address;
if (i2c_SCL_frequency != SCL_frequency_KHz) {
uint16_t fr_sel = round(SCL_frequency_KHz / 100.0);
Copy link
Owner

Choose a reason for hiding this comment

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

I would like to keep that with the round(.), such that frequencies are rounded up to next 100 level. Guess that's ok for NON-OS as well.

@@ -708,13 +726,14 @@ void ICACHE_RAM_ATTR brzo_i2c_read(uint8_t *data, uint8_t nr_of_bytes, boolean r
return;
}

void ICACHE_RAM_ATTR brzo_i2c_start_transaction(uint8_t slave_address, uint16_t SCL_frequency_KHz) {
void ICACHE_FLASH_ATTR brzo_i2c_start_transaction(uint8_t slave_address, uint16_t SCL_frequency_KHz)
Copy link
Owner

Choose a reason for hiding this comment

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

Although wasting a bit of IRAM, I would like to keep the brzo_i2c_start_transaction and its counterpart bzro_i2c_end_transaction in IRAM, i.e. not ICACHE_FLASH_ATTR. As I learned (the hard way), moving from ROM to RAM can take up 3--4 usec, cf. my issue here. There are many use cases, where you want to start i2c transactions right away, thus I would keep RAM attribute.

Copy link
Owner

@pasko-zh pasko-zh left a comment

Choose a reason for hiding this comment

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

Just had a few little comments.
Besides that, tested and works fine.
Thanks!

@pasko-zh pasko-zh merged commit ef53488 into pasko-zh:master Sep 18, 2016
@valkuc
Copy link
Contributor Author

valkuc commented Sep 18, 2016

About round(). I prefer to not use floating point calculation in embedded development when it is possible to implement functionality without it. In your case this can be achieved next way:
uint16_t fr_sel = (SCL_frequency_KHz + 50) / 100;
One more reason why I will not use round on native SDK is because to use it, we also need to link "libm" to or code. For some reason that was fail for me. I did not investigate why it fails.

This was referenced Sep 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants