Beaky2000 / esphome-p1mini

ESPHome external component for reading P1 data from (Swedish) electricity meters
MIT License
128 stars 26 forks source link

RTS always high will not compile #20

Closed gunstr closed 11 months ago

gunstr commented 11 months ago

@Beaky2000 - First of all a big thank you for sharing this code.

I have been running the p1mini for about a year now and it has been working perfect. With my hardware I have the RTS always high and get an output every 10 seconds from the meter.

As you have done some changes I wanted to upgrade to the latest version and then followed the NO-RTS instruction and have replaced id(p1_period), with nullptr,.

However, then system will not compile. I'm getting the error below.

If I keep the number sensor and pass in the id(p1_period), instead it compiles without error and what I can see the reporting in HA is OK. I can of course leave it by that but it would be interesting to understand why the recommended configuration does not work.

In file included from src/main.cpp:78:
src/p1mini.h:38:9: error: 'Number' has not been declared
         Number *update_period_number = nullptr,
         ^~~~~~
src/p1mini.h:178:5: error: 'Number' does not name a type
     Number const *const m_update_period_number{ nullptr };
     ^~~~~~
src/p1mini.h: In constructor 'P1Reader::P1Reader(esphome::uart::UARTComponent*, int*, esphome::gpio::GPIOSwitch*, esphome::gpio::GPIOSwitch*, esphome::gpio::GPIOBinarySensor*)':
src/p1mini.h:45:11: error: class 'P1Reader' does not have any field named 'm_update_period_number'
         , m_update_period_number{ update_period_number }
           ^~~~~~~~~~~~~~~~~~~~~~
src/p1mini.h: In member function 'long unsigned int P1Reader::GetUpdatePeriod()':
src/p1mini.h:183:13: error: 'm_update_period_number' was not declared in this scope
         if (m_update_period_number == nullptr) return 0;
             ^~~~~~~~~~~~~~~~~~~~~~
src/p1mini.h:184:43: error: 'm_update_period_number' was not declared in this scope
         return static_cast<unsigned long>(m_update_period_number->state * 1000.0f + 0.5f);
                                           ^~~~~~~~~~~~~~~~~~~~~~
src/p1mini.h: In member function 'bool P1Reader::CTSAlwaysHigh()':
src/p1mini.h:189:16: error: 'm_update_period_number' was not declared in this scope
         return m_update_period_number == nullptr;
                ^~~~~~~~~~~~~~~~~~~~~~
*** [/home/gunnar/Documents/Dator/HomeAssistant/configSkene/ESPhome/.esphome/build/p1mini/.pioenvs/p1mini/src/main.cpp.o] Error 1
========================= [FAILED] Took 72.08 seconds =========================
Beaky2000 commented 11 months ago

What version of ESPHome are you using? I am getting another error with the latest versions and I haven't figured out what the problem is, so I am sticking with 2023.9.0 for the time beeing, which works for me. So I would suggest trying this version.

gunstr commented 11 months ago

I'm currently using 2023.10.1. I will try with 2023.9.0 tomorrow and let you know.

gunstr commented 11 months ago

Now I have done some more testing.

For the record I noted that with 2023.10.1 the system did not work well also when keeping the id(p1_period), in the lambda. It looked good for a few minutes with updated values every 10s but then all values froze for 30-60 minutes. Then a burst of updated values agian for a few minutes, another freeze and so on.

However, when compiling with 2023.9.0 it all looks good. It now compiled with the nullptr in the lamda witout errors.

I then removed the number template and recompiled - and the error was back. Cleaning the build files did not help but if I added back the number sensor it worked again. I even tried to remove the .esphome folder completely but the code does not comile without having the number template in the yaml file.

So also if the number template is not even mentioned in the lambda it has to be in the yaml file for the code to compile.

I have tested the same with 2023.10.1 with the same result. I really want to stay on this release as there is a long standing issue with Dallas sensors that seems to be correcten in this release an on.

So althoug I cannot explain why I think I have a working system for now. Probably no reason to dig deeper in the root cause - better to try to focus on converting the code to an external component and hope for the best.

Beaky2000 commented 11 months ago

Yes, exactly, until I figure out the external component thing, I would prefer not to change what I have unless I really have to!