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

Can you make this code non-blocking? #116

Open
ham3dt opened this issue Nov 29, 2023 · 11 comments
Open

Can you make this code non-blocking? #116

ham3dt opened this issue Nov 29, 2023 · 11 comments

Comments

@ham3dt
Copy link

ham3dt commented Nov 29, 2023

Is it possible to make this code non-blocking?

@Nismonx
Copy link

Nismonx commented Nov 29, 2023

@ham3dt that's what pull requests are for!?

@ham3dt
Copy link
Author

ham3dt commented Nov 29, 2023

@Nismonx Sorry I'm new to this, can you do it though? is it possible?

@Nismonx
Copy link

Nismonx commented Nov 29, 2023

@ham3dt I think you are missing the point.

Using chatgpt to question a devs work and asking him to put it right not even knowing if your source is correct.

And no, I can't do it. It's working fine for me.

Is there a reason for this request?
In othere words, are you expecting an issue that made you look in to the solution?

@ham3dt
Copy link
Author

ham3dt commented Nov 29, 2023

@Nismonx Please don't get me wrong, I'm not questioning your work and I appreciate the library you provided for free, thank you.

There's another library which uses non-blocking code:
https://github.com/vortigont/pzem-edl

I was wondering if you can please do a few changes to make this library non-blocking as well.

I also edited the request content, sorry if it was offending.

@Nismonx
Copy link

Nismonx commented Nov 29, 2023

@ham3dt , for the record this is not my work.....

But you getting there now.

Over to mandulaj.

@ham3dt
Copy link
Author

ham3dt commented Nov 29, 2023

@Nismonx I thought you're at least a contributor 😄

@mandulaj Pretty please?👉👈

@sergiomnb
Copy link

sergiomnb commented Nov 30, 2023

hi all.
@Nismonx
@ham3dt that's what pull requests are for!?
I don't think this is the way to behave ,at least it wasn't once ,maybe habits have changed.

@ham3dt
Is it possible to make this code non-blocking?

it would be interesting to understand what problems you have with the library with the non-blocking code and why you don't use that.
If it was because it only works with the ESP32 while you want to use it with another device I could tell you that no ,this one to use other devices fails to implement a type of real time communication as you can do with the ESP32.
Otherwise better clarify your goals,the devices you are using,the code you are using and why you need the non-blocking code so that we can help you.

Sergio

@ham3dt
Copy link
Author

ham3dt commented Nov 30, 2023

@sergiomnb How did I behaved? you like making drama out of nothing?

Yes, that library is for ESP32 and I'm using this of course for Arduino. I'm planning to do some math and averaging and TBH this library is a bit slow.

So I appreciate it if you can do some optimizations to the code, as I'm reading PZEM004Tv30.cpp file there are a lots of "to do" that developers commented such as:

PZEM004Tv30::~PZEM004Tv30()
{
    // TODO: Remove local SW serial
    // This is not the correct way to do it. 
    // Best solution would be to completely remove local SW serial instance and not deal with it.
#if defined(PZEM004_SOFTSERIAL)
    if(this->localSWserial != nullptr){
        delete this->localSWserial;
    }
#endif
}

@mandulaj
Copy link
Owner

Hello all, calme vous guys! Let's keep this professional.

To @ham3dt request:

Is it possible to make this code non-blocking?

Yes, in theory, everything is possible. But at what cost?

This library barely supports both basic Arduinos as well as ESP8266 and ESP32. This means any time you want to make a change, you have to verify it on all platforms.

I would be tremendously happy to see a DMA based asynchronous serial read, but, that would require quite significant rework. If you find the time, you are welcome to have a look into this and implement a fully asynchronous receive. However good luck making it cross-platform.

Is this a lazy answer? Yes, but the fact is, I don't have a lot of free time to implement this functionality. And if the current implementation does the job well 90% of the cases, I won't invest my time to cover the 10% edge case scenarios.

More importantly, I need to see clear arguments, why you can't get away with using the synchronous read.

And as @sergiomnb already said, I have a feeling that this is a XY problem. What exactly is the issue that you are running into besides ("the library being slow"). Perhaps there is an alternative solution to your problem to what ChatGPT suggested.

X

@sergiomnb
Copy link

@ham3dt

@sergiomnb How did I behaved? you like making drama out of nothing?

It wasn't meant for you.

@TIBI4
Copy link

TIBI4 commented Sep 3, 2024

In my case I'm using this library to read values while another part of the program controls the speed for a stepper motor. The problem is that the library takes so much time that the motor doesn't get the signal for the next step at the right time and you can hear it miss-stepping.

I was also working with another hardware were the library had the same issue. It was a temperature sensor, but there was an implementation for it that let the library be less blocking.

I'm on Arduino MEGA.

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

No branches or pull requests

5 participants