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

Support Serial1 on ESP32 #7

Closed
strange-v opened this issue Feb 1, 2020 · 5 comments
Closed

Support Serial1 on ESP32 #7

strange-v opened this issue Feb 1, 2020 · 5 comments

Comments

@strange-v
Copy link

Hi, thank you for the great library.

I would like to use it with ESP32 HW Serial1 on custom pins (HW serial could be remapped to almost any pins on this MC) however, the library reinitializes serial interface during the init.
I can fix code in the library, but it isn't a good way.

#ifdef HAS_HW_SERIAL
    static_cast<HardwareSerial*>(uart)->begin(9600, SERIAL_8N1, 16, 17);
#endif

As for me, the library shouldn't reconfigure the interface, just use it, though probably this code was added to simplify usage.
Any thoughts about some kind of workaround?

@avaldebe
Copy link
Owner

avaldebe commented Apr 15, 2020

If I remember correctly there was an problem with Serial1 on the esp32, so I flagged as unsupported**. BTW, Serial2 is on pins 16 (RX) and 17 (TX).

In any case, maybe I can do something better now.
The call to begin from within the is needed in order to hide the hard/soft serial.
Maybe something like the following (with all the #ifdef required) can achieve what you want.

void SerialPM::init(uint8_t rx=0, uint8_t tx=0) {
  if (hwSerial) {
    if (rx & tx) {
      static_cast<HardwareSerial *>(uart)->begin(9600, SERIAL_8N1, rx, tx);
    } else {
      static_cast<HardwareSerial *>(uart)->begin(9600, SERIAL_8N1);
    }

I know I took very long to answer, but still need think to think about it.
In the meantime, I'll enable SoftwareSerial on the ESP32.

@avaldebe
Copy link
Owner

avaldebe commented Apr 15, 2020

I won't enable SoftwareSerial on the ESP32, so could use the SerialPM(sensor, rx, tx) constructor as follows:

#ifdef HAS_SW_SERIAL
  SerialPM(PMS sensor, uint8_t rx, uint8_t tx) : pms(sensor)
  {
    SoftwareSerial serial(rx, tx);
    uart = &serial;
    hwSerial = false;
  }
#elif defined(ESP32)
  SerialPM(PMS sensor, uint8_t rx, uint8_t tx) : pms(sensor), rx(rx), tx(tx)
  {
    uart = &Serial1;
    hwSerial = true;
  }
#endif

@avaldebe avaldebe changed the title Use preconfigured serial interface Support Serial1 on ESP32 Apr 16, 2020
@strange-v
Copy link
Author

It's okay as a workaround.
However, do you see any disadvantages of using Stream * stream to work with both software and hardware serial? I still think that the best option is to use a preconfigured interface by the user, so you don't have care about the type (software/hardware) and pins.
Of course, it is up to you, but I use this approach here without any issues.

@avaldebe
Copy link
Owner

I see your point. However, it is "standard practice" to hide the wire.begin call on I2C sensor libraries, so I see no problem to do the same here.

On a library I prefer the "tell me what you want" approach over the "tell me how to do it".
The SerialPM(sensor, serial) constructor set the sensor on the pins of a predefined serial port, the SerialPM(sensor, rx, tx) constructor sets the sensors on whatever pins you want. The fact that it is a HardwareSerial or a SoftwareSerial behind should be handled by the library not the user.

@strange-v
Copy link
Author

it is "standard practice" to hide the wire.begin call on I2C sensor libraries

As with UART, i2c pins can be remapped...
But, in general, I understand your point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants