paulvha / sps30

Sensirion SPS30 driver for ESP32, SODAQ, MEGA2560, UNO,UNO-R4 ESP8266, Particle-photon on UART OR I2C coummunication
GNU General Public License v3.0
67 stars 27 forks source link

Arduino Zero (SAMD21) Issues #9

Closed ysmilda closed 4 years ago

ysmilda commented 4 years ago

I've run into a couple issues while using this library on an Arduino Zero and Adafruit Metro M0 board:

printf.h: #define _Stream_Obj_ Serial could be changed to #define _Stream_Obj_ SERIAL_PORT_MONITOR to work on most platforms

sps30.cpp: On this platform the hardware Serial (D0 and D1) are called Serial1. This port is excluded by the ifdefined rules. My proposal is to change #if defined(__AVR_ATmega32U4__) to #if defined(__AVR_ATmega32U4__) || defined(ARDUINO_ARCH_SAMD)

paulvha commented 4 years ago

Thanks for the feedback, but I am not planning to make these changes. See below the reasons

with respect to Printf.h, SERIAL_PORT_MONITOR is not defined in all the board variants. I have about 10 different libraries for boards & many variants and only 5 has SERIAL_PORT_MONITOR ALL while of them have Serial. On those 5 where SERIAL_PORT_MONITOR is defined it is done as :

define SERIAL_PORT_MONITOR Serial.

Serial1. The different SMDXX chips have a number of SERCOM (SAMD21x has 6, SAMD51x has 8) which can be flexible mapped to a number of different pins of the chip. It looks indeed that on the zero they made Serial1 to D0 and D1 in variants.h, but on other boards with SAMD chip a SERCOm is mapped to other pins. Hence it is not a good idea to exclude Serial1 for all the SAMD based architecture boards.

ysmilda commented 4 years ago

I was indeed too quick to judge about the SERIAL_PORT_MONITOR.

I would like to make the case of passing a UART or Wire object to the begin function. By overloading the begin function this can be handled nicely. This way the user could use all the ports and also use self defined ports on SAMD devices.

The basis could look something like:

begin(TwoWire *theWire)
{
_i2c = theWire;
_i2c->begin();
}

begin(Uart *serial)
{
_serial = serial;
_serial->begin(115200);
}
paulvha commented 4 years ago

Thanks for the the advice. With each new project I look how to build it and learn new aspects. I have build a number of libraries different ways. With this project I decided to apply a different way to select the communication channel. It works on the many different boards that I had planned at the start. After posting on github, many people have downloaded and used this library and examples, on many different boards. Often, most of the time, it was straightforward, sometimes more work was needed. I helped where I was requested and made adjustments where necessary to make it to work. If I would have known how many people would use this library, I might have taken a different approach but at this moment I am not planning to change the approach.

That said.. the advantage of the source code available in open source, is that you can adjust it for yourself to the way you want it.

ysmilda commented 4 years ago

I'm aware that I can fork this and make my own version and I might do that, I was just curious if there was any interest to include this other way of selecting communication channels in the library, maybe as a 2.0 release or something like that.

It would open it up to all the benefits the SAMD platform brings.

ysmilda commented 4 years ago

I've made a fork to incorporate these changes which can be found here: https://github.com/AdvancedClimateSystems/sps30/

The changes I've made: