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

fix Hardware Serial for esp32 #31

Merged
merged 3 commits into from
Jul 15, 2021

Conversation

nguyenhuudamy
Copy link

No description provided.

@mandulaj
Copy link
Owner

Hi @nguyenhuudamy Thanks for the PR. However I don't like the idea of changing an established API. It would for sure break my projects not to mention anyone who already uses this library... Perhaps we can overload the function or wrap it in a #if #else to fix it just for the esp32 platform while retaining the Arduino compatible API.

@mandulaj mandulaj added this to the Release of v1.1 milestone Jan 23, 2021
@ManuelJimenezBuendia
Copy link

What about something like this?
.h
#ifdef ESP32
PZEM004Tv30(HardwareSerial* port, int rx_pin, int tx_pin, uint8_t addr=PZEM_DEFAULT_ADDR);
#endif

.cpp
#ifdef ESP32
PZEM004Tv30::PZEM004Tv30(HardwareSerial* port, int rx_pin, int tx_pin, uint8_t addr)
{
port->begin(PZEM_BAUD_RATE, SERIAL_8N1, rx_pin, tx_pin); //Baud rate, parity mode, RX, TX
this->_serial = port;
this->_isSoft = false;
init(addr);
}
#endif

Copy link
Owner

@mandulaj mandulaj left a comment

Choose a reason for hiding this comment

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

So I added the requested #if def and modified the examples in order to be compatible. It has compiled however I have no way of testing it on any board at the moment. Could anyone with access to an ESP32 board try this? Maybe @ASMotionLab if you have time.

PZEM004Tv30.h Show resolved Hide resolved
@loopyengineeringco
Copy link

Thanks @mandulaj, looks great at a glance - I will check it out this evening, test with my ESP32 and report 👍

@mandulaj
Copy link
Owner

I managed to get hold of an ESP32 and it seems to work fine... So I am ready to merge right away.

Copy link
Owner

@mandulaj mandulaj left a comment

Choose a reason for hiding this comment

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

I added the ESP32 switch and it works fine now

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