search

psvanstrom / esphome-p1reader

ESPHome custom component for reading P1 data from electricity meters.
MIT License
251 stars 106 forks source link

Optimize performance (with reworked code structure) #79

Open cadwal opened 10 months ago

cadwal commented 10 months ago

This quickly got out of hand but began when ESPHome started delivering warnings like these:

[W][component:204]: Component took a long time for an operation (0.06 s). [W][component:205]: Components should block for at most 20-30ms.

Irritating so how to fix?

Well initial, but wrong, assumption was that using loop and just continuously polling was the culprit. Actually it was the time to do each iteration of loop it was complaining about...

Having moved to a polling component with a calculated polling interval based on the rx buffer size I realized this when the warnings were identical. But the change should have the benefit of not busy-waiting the cpu and using less power.

So what does cause this then?

One thing is (excessive) logging, this is fixed by having only essential logging as Info, all the rest as debug and then setting loglevel INFO in the config

Another thing is publish_state that apparently also takes time, I think noone at Nabu Case intended a sensor to have this many values... but, here it was possible to move the publish into a separate "time slice" from the fetch and parse stage and then split it into two parts to get the time for each slice down to < 30ms.

Even so there still were frequent complaints for the read part so what to optimize further?

atof could be changed into a simpler variant since the fixed format nature of the P1 data is not so complex. atof time now take about a third of the regular implementation.

Even so there still were less frequent complaints so what to optimize further?

Change the obis parser from O(n) to O(1) (where n is the number of possible obis values) so the overall parsing time is O(n) rather than O(n^2) where n is the number of actual obis values in the message.

Now we are down to almost no warnings about time used...

So here I went to do a PR and realized that "Oh, no!" (https://www.youtube.com/watch?v=sP7njpTd06I) the code has changed...

Skip the whole thing or?

But with some more code rearranging, here we are.

Note that I have NOT tested the HDLC version since I only have the newer standard here (using my Slimmelezer). I did switch it away from the mix of EspHome and Arduino serial access. There is still a compile warning in the HDLC version around the use of sprintf and limited but in reality ok buffer for obis codes. And the HDLC version does not use atof at all...

newlund commented 1 week ago

I had a lot of CRC errors and a lot of "Component took a long time for an operation" messages after adding a P1 splitter (https://www.homewizard.com/shop/active-p1-splitter/)

Tested your PR and the problems are gone. Thanks! :)

However, I get data every 1s with my Sagemcom T211. I got that with the original code as well.

Can I introduce a delay for 10s somewhere to get a 10s interval between samples?

Edit: Realize now that it is probably not possible since the RTS is always high. Added this to all sensors in my yaml instead:

filters:
      - throttle: 9.5s

Seems to work

cadwal commented 1 week ago

However, I get data every 1s with my Sagemcom T211. I got that with the original code as well.

Can I introduce a delay for 10s somewhere to get a 10s interval between samples?

Edit: Realize now that it is probably not possible since the RTS is always high. Added this to all sensors in my yaml instead:

filters:
      - throttle: 9.5s

Seems to work

Yeah, I was going to say that is the only way to do it I have heard about when RTS is always high due to HW setup.