search

Sensirion / arduino-sps

Arduino library for Sensirion SPS30
Other
47 stars 16 forks source link

Rev5 does not work with ESP8266 #4

Closed michapr closed 5 years ago

michapr commented 5 years ago

Thank you for the new library. The provid3ed cde does not work with ESP8266.

Such registers like PORTD and TWSR are not defined here and so the sample code cannot be compiled.

In file included from D:\...\packages\esp8266\hardware\esp8266\2.4.1\cores\esp8266/Arduino.h:255:0,

                 from D:\....\libraries\arduino-sps-master\i2c_master_lib.cpp:64:

D:\....\libraries\arduino-sps-master\i2c_master_lib.cpp: In member function 'void I2C::begin()':

D:\....\libraries\arduino-sps-master\i2c_master_lib.cpp:98:9: error: 'PORTD' was not declared in this scope

     sbi(PORTD, 0);

Maybe you can do it compatible for ESP too?

Thanks! Michael

abrauchli commented 5 years ago

Hi @michapr Although it's good to know, did you mean to post this to https://github.com/DSSCircuits/I2C-Master-Library/issues ? I'll close the issue here since it's a third-party library issue.

michapr commented 5 years ago

Anyway it would be good to add this info to the Readme while it is not working... Library is 7 years old - don't think that there is a support for that...

Michael

winkj commented 5 years ago

thanks for the report, I believe PORTD et al might be Atmel specific. I'll have a look.

@abrauchli I'll reopen this for now to keep track of this

winkj commented 5 years ago

From what I can see in the ESP8266 implementation, the buffer limit in Wire.h is in fact 128 bytes, and it's the same for ESP32.

So I think this should be a straight forward fix: On ESP*, use our standard Arduino implementation (https://github.com/Sensirion/embedded-common/blob/master/hw_i2c/sample-implementations/arduino/sensirion_hw_i2c_implementation.cpp) and #ifdef out the i2c master library.

I don't have access to an ESP8266 board right now, so will have to test later. @michapr if you want to try this and report back, that would be great. I'd simply replace sensirion_hw_i2c_implementation.cpp with the one linked above, and delete i2c_master_lib.h and i2c_master_lib.cpp

michapr commented 5 years ago

I know that it is in new lib for ESP 128bit - I made there the request to add it in new lib :) But in all "older" libs it will not work. The last lib for ESP have some issues, that's why not all users can move.

winkj commented 5 years ago

Thanks for the further explanation @michapr. On top of the issues you mention, the current "stable" release does not include the change yet, so this is not a viable solution. [Correction 2019-04-11: It looks like the current stable version does indeed include that change]

Looking into this a bit further, it appears that the ESP families don't have any hardware I2C (https://bbs.espressif.com/viewtopic.php?p=5449&sid=61b456f9b1cbce155cb1fc88bf2df028#p5449). So short of rewriting the ESP SW I2C stack, I don't think we can support this cleanly.

I'll look into restricting this library to AVR architectures, and add a comment, and then add ESP* once the libraries are updated and stable.

winkj commented 5 years ago

v0.0.5 will support ESP8266 through the Wire library, as long as the user is using the current stable version of the ESP8266 Arduino core

winkj commented 5 years ago

v0.0.5 will support limited functionality - as allowed by the underlying platform - on ESP8266 2.3.0 2.4.0, 2.4.1 and 2.4.2; I hope that will cover the users that can't switch to 2.5.0 yet.

Code is in the limit_i2c_buffer branch and is pending review

winkj commented 5 years ago

Fixed in v0.0.5