arduino-home-assistant icon indicating copy to clipboard operation
arduino-home-assistant copied to clipboard

ArduinoHA 2.0.0 🔥

Open dawidchyrzynski opened this issue 3 years ago • 2 comments

I'm currently working on a new major version of the library that's basically a ground refactor of the previous implementation. The main goal of this branch was to refactor the way how entities are serialized in order to simplify and standardize the codebase.

In order to improve the general quality of the library, I have decided to cover each part of the library with unit tests and write detailed documentation describing the library in each tiny detail.

Stay tuned ✌️

dawidchyrzynski avatar Jun 04 '22 11:06 dawidchyrzynski

Tried to use, but getting

'HASensor' does not name a type

Same for if i try and use your example

netmindz avatar Jun 14 '22 19:06 netmindz

@netmindz The branch is still a work in progress. Some of the entities are not implemented yet (like HASensor).

dawidchyrzynski avatar Jun 14 '22 19:06 dawidchyrzynski

Roughly what % are you through this rewrite? Wondering if it's worth me trying to get my much smaller patch to handle selective update working. It's real pain in the arse in your main project to litter with code to keep track there of if the value has changed before calling update

netmindz avatar Sep 07 '22 12:09 netmindz

@netmindz The rewrite is finished in 80%. Can you explain what you exactly want to achieve?

dawidchyrzynski avatar Sep 08 '22 18:09 dawidchyrzynski

@dawidchyrzynski HABinarySensor::setState only actually sends message if new value != previous value, so in your code of you main app you can just call setState and be sure that you will get immediate MQTT if the value changes. Last update time in HA is the same as last state change.

This is only for the BinartSensor, my PR tried to make that the same for other sensor types too.

Without this you either flood MQTT with the same value over and over, have to put the actual update values all in block of code within your main loop that only triggers every X seconds (adding lag to responsiveness) or for every value you want to manage, have code like the following

if(temp != previousTemp) { tempSensor.setState(temp); previousTemp = temp; }

Which clearly gets messy very quickly

E.g https://github.com/netmindz/balboa_GL_ML_spa_control/blob/1b5fcb801037b1ef099e013904b7514a399c63bb/sensor/sensor.ino#L522

netmindz avatar Sep 09 '22 12:09 netmindz

@netmindz This PR introduces a new implementation for the sensor entity. You can checkout into the serializer-refactor branch and try a new implementation. Basically, the sensor will be split into three different classes:

  • HASensor - textual sensor (no data retention)
  • HASensorInteger - integer numbers (with data retention - comparing states)
  • HASensorFloat - floating point numbers (with data retention - comparing states)

This implementation provides the smallest utilization of the RAM and flash. Here is the changelog that describes all breaking changes in the API: https://github.com/dawidchyrzynski/arduino-home-assistant/blob/serializer-refactor/CHANGELOG.md

Alternatively if it doesn't meet your requirements you can wrap the HASensor (or any other class) with your own class and implement the logic you need. For example:

class MySensor {
public:
    MySensor(): _sensor("uniqueName") {
        _sensor.setName("Pretty name");
        _sensor.setIcon("mdi:home");
    }

    void doSomeStuff() {
        // some custom logic
    }
private:
    HASensor _sensor;
};

dawidchyrzynski avatar Sep 09 '22 12:09 dawidchyrzynski

I am trying to create my own smart home, fortunately I found this library, because it allows me to significantly reduce the programming time of my own smart home based on arduino. I am very grateful and I hope you will continue the great work you are doing. I'm looking forward to the new release because it looks very promisnig. Are you planning to support HALight? Are you able to say how much time you need to complete the release?

patryk-zielinski93 avatar Sep 12 '22 13:09 patryk-zielinski93

Hi @patryk-zielinski93,

The new version is going to support all available device types, including HALight. I think the realistic date for the release is early December.

dawidchyrzynski avatar Sep 12 '22 13:09 dawidchyrzynski

I just discovered you are using the EpoxyDuino mock environment emulation for debugging and that's great.

You may might find useful to also try the mock environment included in esp8266/Arduino. It is not as portable as EpoxyDuino is, but it allows to use native pubsubclient library to test with.

The following bash script compiles and run your nodemcu example.

diff --git a/examples/nodemcu/onhost b/examples/nodemcu/onhost
new file mode 100755
index 0000000..1f50309
--- /dev/null
+++ b/examples/nodemcu/onhost
@@ -0,0 +1,31 @@
+#!/bin/bash -ex
+
+if [ -z "${ESP8266ARDUINO}" ]; then
+    echo "\$ESP8266ARDUINO must point to esp8266/Arduino repository root directory"
+    exit 1
+fi
+
+VALGRIND=${VALGRIND-valgrind}
+pwd=$(pwd)
+code=${pwd##*/}
+
+# using native pubsubclient
+ULIBDIRS=${pwd}/../..:${pwd}/../../../pubsubclient
+
+# sources in other directories
+SRC=$(echo ${pwd}/../../src/device-types/*cpp ${pwd}/../../src/utils/*cpp)
+
+# build
+cd ${ESP8266ARDUINO}/tests/host
+make -j 8 D=1 V=1 FORCE32=0 ULIBDIRS=${ULIBDIRS} USERCXXSOURCES="${SRC}" ${pwd}/${code}
+
+# run
+set +e
+${VALGRIND} ${ESP8266ARDUINO}/tests/host/bin/${code}/${code} "$@"
+ret=$?
+
+# fix console at exit
+set +x
+stty sane
+
+echo ret=${ret}

I currently get this error, but I'm not going to complain with a ongoing work :]

==2601846== Mismatched free() / delete / delete []
==2601846==    at 0x4845B0F: operator delete(void*, unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==2601846==    by 0x13464B: HADevice::~HADevice() (HADevice.cpp:45)

d-a-v avatar Sep 15 '22 13:09 d-a-v

@d-a-v It looks interesting. I will have a look at this once the main work is finished. Can you comment the HADevice.cpp:45 line and try to build the lib again?

dawidchyrzynski avatar Sep 15 '22 13:09 dawidchyrzynski

The error disappeared :+1:

On termination, there are 4 leaks, 3 of them come from the mocking subsystem, the 4th is detailed below.

==2630715== 
==2630715== HEAP SUMMARY:
==2630715==     in use at exit: 325 bytes in 4 blocks
==2630715==   total heap usage: 32 allocs, 28 frees, 2,249,513 bytes allocated
==2630715== 
==2630715== 13 bytes in 1 blocks are still reachable in loss record 1 of 4
==2630715==    at 0x4844293: operator new[](unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==2630715==    by 0x13BA3D: HAUtils::byteArrayToStr(unsigned char const*, unsigned short) (HAUtils.cpp:44)
==2630715==    by 0x13468F: HADevice::setUniqueId(unsigned char const*, unsigned short) (HADevice.cpp:55)
==2630715==    by 0x10DED2: setup (nodemcu.ino:29)
==2630715==    by 0x10F478: main (ArduinoMain.cpp:308)

Once again, this is not to blame anyone or a work-in-progress particularly. On top of that, on Arduino, some buffers may be allocated and never released anyway. What's interesting is that mocking environments and valgrind help debugging mcu code. Speaking of esp8266, this one was designed for CI originally, and later helped to debug internal and external network libraries, like yours is.

d-a-v avatar Sep 15 '22 15:09 d-a-v

@d-a-v This memory leak is caused by the line you just commented. Can you try to uncomment it and replace it with delete[] _uniqueId;. It should still build and fix the leak :)

dawidchyrzynski avatar Sep 15 '22 15:09 dawidchyrzynski

@d-a-v Also please make sure you're working on the latest commit. I was fixing memory leaks yesterday so maybe the problem you found is already solved.

dawidchyrzynski avatar Sep 15 '22 15:09 dawidchyrzynski

This indeed solved both issues :+1: (using current version of this PR)

diff --git a/src/HADevice.cpp b/src/HADevice.cpp
index 4642fda..e30416d 100644
--- a/src/HADevice.cpp
+++ b/src/HADevice.cpp
@@ -42,7 +42,7 @@ HADevice::~HADevice()
     }
 
     if (_ownsUniqueId) {
-        delete _uniqueId;
+        delete [] _uniqueId;
     }
 }

d-a-v avatar Sep 15 '22 15:09 d-a-v