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

Constructor overload for better ESP32 support #55

Closed
wants to merge 2 commits into from

Conversation

vortigont
Copy link
Contributor

  • example for esp32 with custom pin mapping

Closes #20

Signed-off-by: Emil Muratov gpm@hotplug.ru

+ example

Closes mandulaj#20

Signed-off-by: Emil Muratov <gpm@hotplug.ru>
Signed-off-by: Emil Muratov <gpm@hotplug.ru>
@mandulaj
Copy link
Owner

Hey @vortigont , I finally found some time for closing all the old PRs, including #31 and now noticed that you have also provided a similar fix here.
I will therefore not be merging your Constructor overload however I could cherry pick your Fix for the compiler warning.

@mandulaj mandulaj changed the base branch from master to rel1.1 July 16, 2021 13:41
@mandulaj
Copy link
Owner

In the new update, I cherry picked your change, However there is now a different error showing up.

PZEM004Tv30.cpp: In destructor 'PZEM004Tv30::~PZEM004Tv30()':
PZEM004Tv30.cpp:142:9: warning: deleting object of polymorphic class type 'SoftwareSerial' which has non-virtual destructor might cause undefined behavior [-Wdelete-non-virtual-dtor]
  142 |         delete this->localSWserial;

So this hasn't really improved the situation. As I have already suggested, the best thing would be to move away from local SoftwareSerial instance and instead require one to be passed as reference during the initialization. This way, we don't have to handle any destruction.

@mandulaj mandulaj closed this Jul 16, 2021
@mandulaj mandulaj deleted the branch mandulaj:rel1.1 July 16, 2021 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ESP32 Support
2 participants