pihome-shc / PiHomeHVAC

Next Generation Smart Thermostat
Other
11 stars 8 forks source link

Home Assistant integration #78

Open pihome-shc opened 3 years ago

pihome-shc commented 3 years ago

@JSa1987 made some changes to the API to allow a better integration with Home Assistant. The idea is for PiHome to still run the whole logic but to be able to monitor the status from Home Assistant.

getTemperature.php has been changed to return also the sensor battery status, it now returns:
{"success":true,"state":"19.00","datetime":"2021-02-28 15:23:34","bat_voltage":"2.50","bat_level":"43.00"}

getBoiler.php has been added to return the boiler status:
{"success":true,"state":"OFF"}

This allows the temperature of the zones, boiler status and battery info to be monitored from a dashboard in Home Assistant. In addition, the Zone Boost can be triggered from the Home Assistant dashboard.

@JSa1987 can write a little how to on how to setup the Home Assistant side to work with this.

pihome-shc commented 3 years ago

this document is published on PiHome website, https://bit.ly/2QAICFY thank you @JSa1987 for working on documentation

twa127 commented 3 years ago

Hi,

For PiHomeHVAC I would like to change the name of the getBoiler.php API to getController.php as this makes more sense for MaxAir is this acceptable?

twa127 commented 3 years ago

Additionally I think that for MaxAir getTemperature.php needs to be changed as we now have temperature sensors which are not attached to zones.

dvdcut commented 3 years ago

@twa127 i dont use home assistant myself but i agree, with you, this will make naming convention right.

scottagecheeseandcrackers commented 3 years ago

Big thanks to all - this HA integration coupled with the Maxair update is some piece of work.

One suggestion/request...I have set up some room sensors which provide temperature but are not themselves a zone. Can the '/api/getZoneStatus.php?..,' requests be updated to to 'getSensor...' aswell?

twa127 commented 3 years ago

Already there - api/getTemperature.php

scottagecheeseandcrackers commented 3 years ago

Bingo....just what I was looking for...thanks @twa127

JSa1987 commented 3 years ago

@scottagecheeseandcrackers have a look at #201 this implements the integration with Home Assistant over MQTT: not configuration needed on HA side and is much more powerful!!! You get a climate entity in Home Assistant for each zone in MaxAir, plus a lot of monitoring sensors for you MaxAir RPi.

If you have the Mosquito MQTT broker running on your Home Assistant all you need to do is setup a new MQTT account for MaxAir, enter the MQTT connection details in MaxAir and then run the add_on/HomeAssistant/install.sh script. See [here](https://github.com/pihome-shc/PiHomeHVAC/tree/main/add_on/HomeAssistant) for details.

As of now this does not include the temperature sensors not associated with a zone, but I think this can be easily added. I will have a look into this.

dvdcut commented 3 years ago

@JSa1987 maxair already have add-on installation process, i suggest to add add_on/HomeAssistant/install.sh in install software section

JSa1987 commented 3 years ago

@dvdcut I noticed the option in the web interface, but haven't figured out what needs to be done to add a new installer like add_on/HomeAssistant/install.sh. Can you guide me in the right direction?

twa127 commented 3 years ago

@JSa1987 you have already done it, by adding install.sh to the add-on directory it will be picked up by Settings/System Configuration/Install Software

pihome-shc commented 3 years ago

@JSa1987

boiler.php is replaced by controller.php

install.sh

#!/bin/bash

echo "Installing Phyton modules"
REQUIREMENTS=requirements.txt
sudo pip3 install -r $REQUIREMENTS

echo "Editing boiler.php to disable override when the current schedule for the zone end"
sudo sed -i.bak '/HA-Integration/s/^#//g' /var/www/cron/boiler.php
echo "Backup of original version of boiler.php created (/var/www/cron/boiler.php)"

echo "Creating service for auto start"
sudo cp HA_integration.service /etc/systemd/system/HA_integration.service
sudo systemctl enable HA_integration.service

echo "Starting the service"
sudo systemctl start HA_integration.service
twa127 commented 3 years ago

yes MaxAir has changed boiler.php to controller.php

twa127 commented 3 years ago

@JSa1987 I've modified your install.sh to change boiler.php to controller.php and add a name and description header

JSa1987 commented 3 years ago

@twa127 that line was a left over when I ported this from PiHome to MaxAir. In MaxAir the change to controller.php is not needed. I will upload an amended install.sh script later today.

JSa1987 commented 3 years ago

@scottagecheeseandcrackers I have submitted a pull request (#206) that add support also to stand-alone temperature and humidity sensors to the implementation over MQTT. Give it a try and let me know if you have any feedback.

@pihome-shc / @twa127 #206 fixes also the install.sh script.

scottagecheeseandcrackers commented 3 years ago

@scottagecheeseandcrackers I have submitted a pull request (#206) that add support also to stand-alone temperature and humidity sensors to the implementation over MQTT. Give it a try and let me know if you have any feedback.

@pihome-shc / @twa127 #206 fixes also the install.sh script.

Sorry took me some time to test this - Worked a treat for me. Hats off. Only query - the stand alone temperature sensor gives me less data than the zone data. (Mainly battery info as they aren't linked to a zone)? I had hoped that this would also work in reverse...ie. you could set up a mqtt type sensor in maxair - and mqtt messages would be logged in messages_in just like any other sensor.

JSa1987 commented 3 years ago

@scottagecheeseandcrackers just to understand you would like to have the battery voltage and percentage added as attributes in HA for the stand alone temperature sensors? It shouldn't be a problem to implement this feature, and I can add it as I'm planning anyways to clean up a bit the code.

With regards to the second request you are talking about having MaxAir using MQTT temperature sensors and MQTT controlled relays? If so, I'm working also on this but this is completely separate from the HA integration. There is a ling discussion on this here #191. I had pushed a first attempt of implementation #212 but there were some concerns on this so it was rolled back. I'm now working to address these concerns and I think I will soon have something that is ready to be tested. If you do have some MQTT sensors (and maybe even relays) it would be great if you could help with the testing and feedback.

scottagecheeseandcrackers commented 3 years ago

@JSa1987 - on request 1 - if possible, it would be great to have temperature, battery and last seen attributes included in each published message.

On request 2 - happy to test what-ever aspect you like. I have some tasmotas running around the house which I have the added DS18x20 probes to. Though I dint have any tasmotas connected to my heating system - I could easily test a relay setup.

JSa1987 commented 3 years ago

@scottagecheeseandcrackers I tired to implement request 1 and made a couple of other adjustments. We now have the "last seen" attributes for all zones and stand alone sensors. In addition, we should have the battery voltage and percentage for stand alone MySensors but I cannot really test this as I don't have a spare MySensors. If you don't mind could you please give this a try and feedback if it work as expected? To test this I would recommend you make a backup of your current script before trying the one attached:

# cd /var/www/add_on/HomeAssistant
# systemctl stop HA_integration.service
# cp HA_HVAC_integration.py HA_HVAC_integration.py.bk

Copy over the new HA_HVAC_integration.py and then try to run it:

# python3 HA_HVAC_integration.py

If you are happy with the testing and want to keep using the new script after terminating it you can re-start the service:

# systemctl start HA_integration.service

HA_HVAC_integration.py.zip

twa127 commented 3 years ago

Can I just state what is the problem for me -

I fully see the value of having Home Assistant integrated with MaxAir but I do believe that it cannot be deployed in its current form. If provided as an Add-On it should install and configure all prerequisites not included in the basic LAMPs platform..

Please take these observations as constructive, I'm more than happy to see Home Assistant integration

JSa1987 commented 3 years ago

@twa127 Home Assistant would be run on a dedicated separate hardware. Home Assistant is a way to control all the different "smart" devices in the house from one centralized interface, and allows create logics to automate the control of such devices. I wouldn't see why a user would expect MaxAir to provide Home Assistant running on the same SBC. For a Home Assistant user MaxAir is a "device" the same way a Hue bulb is or Kodi media player. To give you an idea this page lists the integrations currently available for Home Assistant (i.e. the devices that can be controlled). With #201 MaxAir comes in under MQTT HVAC (for the zones) and MQTT Sensor (for the stand alone sensors).

I see this as the Amazon Echo add-on that assumes that the user has a Amazon Echo device. If the user does not have an Amazon Echo even if he/she install the add-on from the Install Software menu this will not work. It would be unreasonable to expect an add-on for a specific device (Amazon Echo or Home Assistant) to work if you don't have the device.

twa127 commented 3 years ago

I take yor point, but there is a difference - because the Echo is an internet connected device both the fauxmo and homekit services will not fall over it the echo goes off line, this is not the case for the ha_integration service, which will be trying to connect to a non-existent server.

Also I still think my point with regards to user expectations is still valid, a none 'expert' user (me in the case of Home Assistant), I expected to install the Add-On and it would work, that's not quite the same as me not having gone out and purchased an Echo. Perhaps the ReadMe could offer a bit more clarity, I followed the Quick Start steps and got nowhere (because I did not understand that I needed Home Assistant to be running somewhere). Most users will know what an Echo is, I doubt that's the case for Home Assistant.

JSa1987 commented 3 years ago

Ok, I agree that we should specify that this add-one requires the user to have Home Assistant running on a separate device. I will edit the ReadMe to reflect that.

JSa1987 commented 3 years ago

@scottagecheeseandcrackers if you want to try the latest version please use the file attached. I made a few changes. HA_HVAC_integration.py.zip

twa127 commented 3 years ago

sounds good to me, can I also request -

JSa1987 commented 3 years ago

@twa127 I have been working on all the changes you suggested:

Attached are the new files if you want to have another look and feedback. I will test this for a couple of days and if it all works well I will submit a pull request.

HomeAssistant.zip

scottagecheeseandcrackers commented 3 years ago

@JSa1987 - running "...integration.py" generated an error:

root@maxair:/var/www/add_on/HomeAssistant# python3 HA_HVAC_integration.py
Traceback (most recent call last):
  File "HA_HVAC_integration.py", line 111, in <module>
    MA_Sensor_Child_ID.append(row[description_to_index["sensor_child_id"]])
KeyError: 'sensor_child_id'
scottagecheeseandcrackers commented 3 years ago

Sorry should have added....that was the latest zip file I used.

JSa1987 commented 3 years ago

@scottagecheeseandcrackers thank you for the feedback. I found a variable that was getting re-assigned and that would have caused that error in case there are multiple stand-alone sensors in MaxAir. Is that your case?

This should be fixed in the version attached, but I have no way to test it as have only one stand-alone sensor. Please give it a try when possible and let me know if you have any issue. You just need to copy over the HA_HVAC_integration.py file.

HomeAssistant.zip

twa127 commented 3 years ago

@JSa1987 will test as best I can, need to build a HA server, but will do that.

One thing about rpi_bad_power is that it can be installed on OPi okay, so no need for import check, just returns a None value, so can you code around that in get_rpi_power_status()

fyi - multiple stand alone sensors could be a common situation

scottagecheeseandcrackers commented 3 years ago

@scottagecheeseandcrackers thank you for the feedback. I found a variable that was getting re-assigned and that would have caused that error in case there are multiple stand-alone sensors in MaxAir. Is that your case?

This should be fixed in the version attached, but I have no way to test it as have only one stand-alone sensor. Please give it a try when possible and let me know if you have any issue. You just need to copy over the HA_HVAC_integration.py file.

HomeAssistant.zip

Working well:

Here is the the return messages:

image

New attributes added for all sensors to include last seen but battery data noly showing for sensors alloacted to a zone.

Kitchen and MasterBedroom sensors are both MySensors - and neither are allocated to a zone.

JSa1987 commented 3 years ago

@scottagecheeseandcrackers I just noticed that on line 122 there is a space that is not supposed to be there. Try to change MA_Sensor_Type .append(description_to_index_nodes["type"]) to MA_Sensor_Type.append(description_to_index_nodes["type"])

scottagecheeseandcrackers commented 3 years ago

@JSa1987 - line 122 fix implemented - but no change.

Checked by DB - sensor type is defiantly set as 'MySensor'.

image

But it seems that they are only reporting the 'last_seen' attribute:

)
        if MA_Sensor_Type[sensor] == "MySensor":
            payload_str = (
            '{'
            + f'"last_seen": "{sensor_status[1]}",'
            + f'"batt_level": "{sensor_status[2]}",'
            + f'"batt_voltage": "{sensor_status[3]}"'
            + ' }'
            )
        else:
            payload_str = (
            '{'
            + f'"last_seen": "{sensor_status[1]}"'
            + ' }'
            )
JSa1987 commented 3 years ago

@scottagecheeseandcrackers ok, I found the issue. This was on line 122 where I was assigning to MA_Sensor_Type the column index instead of the column value. This is fixed in the attached file.

HomeAssistant.zip

scottagecheeseandcrackers commented 3 years ago

Bingo - All sensors and attributes are reading perfect. image

Working well - I'll do a few more test's - such as publishing a boost command and report back.

scottagecheeseandcrackers commented 3 years ago

@JSa1987 - Okay - initial testing...

I have tried setting a boost but no joy. I have tried publishing the following messages: {"hvac_action":"heating","temperature":"70","aux_heat":"ON"} {"hvac_action":"heating","temperature":"70"} {"aux_heat":"ON"} But no change in MaxAir gui. After a 30 seconds or so - MaxAir just publishes a new message noting no changes.

I have manually run a boost - using the MaxAir Gui - and I get an MQTT response showing the correct values. I just havn't been able to trigger a boost via publishing an MQTT message. Hope this helps.

JSa1987 commented 3 years ago

@scottagecheeseandcrackers I have tested the functionality of triggering the boost function quite extensively and it always work well. Reading you post my understanding is that you are trying to trigger this manually publishing a MQTT message is that correct? As each MaxAir zone is represented in Home Assistant by a climate entity the easiest way to trigger the boost for a zone is from the Lovelace Thermostat card to use the Aux Heat switch. HA_Aux_heat

If for any reason you want to manually send the MQTT message (e.g. in an automation) the the correct way would be as follow (also see the example for the office zone in the screenshot below).

Topic: MaxAir\zonename\aux_command Message: ON

Boost_MQTT

scottagecheeseandcrackers commented 3 years ago

Cheers @JSa1987 - this clears things up. Yes - manually publish messages to this topic did the trick for me.

Apologies - I'm stretching this issue category further away from Home Assistant. Until recently I had HA running on a spare RPI. I recently swapped over to homebridge - as the integration with Google Home Assistant was much much simpler (and free).

I'm probably mis-using your intended integration - but I'm currently re-using the topic messages for each zone - for use as a MQTT Thermostat in Homebridge. (https://github.com/arachnetech/homebridge-mqttthing). This gives me a very slick and easy to control thermostat in Google Home Assistant - that looks like this... Screenshot_20210927-192554_Home.jpg

JSa1987 commented 3 years ago

@scottagecheeseandcrackers we now have support in MaxAir for MQTT sensor and MQTT relays (see #226 and #232). I believe this is one of the features you were talking about above. The setup guide has been updated with instructions on how to set this up.

scottagecheeseandcrackers commented 3 years ago

@JSa1987 - I'm not sure which update may have caused an issue in the last few days - but i've noted that my HA integration service is no longer processing info related to 'zones' - it will only issue messages related to 'sensors'. I have tried restarting the service per the readme. Also have stopped the service and replaced the add_on/Home Assistant folder files with the latest version on github and restarted the servcie - but no joy. Have you any advices on how I could debug this to find the issue? Off the top of my head I wondered if #241 pkill on long running processes could be causing an issue?

JSa1987 commented 3 years ago

@scottagecheeseandcrackers I just checked on my system and the HA integration is still working for all zones. Can you try to stop the service and manually run HA_HVAC_integration.py to see if there is any error message?

# systemctl stop HA_integration.service 
# cd /var/www/add_on/HomeAssistant/
# python3 HA_HVAC_integration.py 
scottagecheeseandcrackers commented 3 years ago

@JSa1987 - here is the output

/var/www/add_on/HomeAssistant# python3 HA_HVAC_integration.py send config message Connected to broker Exception in thread Thread-1: Traceback (most recent call last): File "/usr/lib/python3.7/threading.py", line 917, in _bootstrap_inner self.run() File "HA_HVAC_integration.py", line 158, in run self.execute() File "HA_HVAC_integration.py", line 250, in updateSensors sensor_status = get_sensor(sensor) File "HA_HVAC_integration.py", line 498, in get_sensor if results[description_to_index["payload"]] is None: TypeError: 'NoneType' object is not subscriptable

twa127 commented 3 years ago

@scottagecheeseandcrackers Hi, I can't see any reason #241 would be likely to cause the problem, it only gets used by the code update process if any of the long running script files have been changed, so will only get run once.

twa127 commented 3 years ago

@scottagecheeseandcrackers I'm only guessing but I suspect that the query at line 493 is not returning any rows, if you can send me a copy of your database I might be able to see why

scottagecheeseandcrackers commented 3 years ago

Cheers @twa127 - I'm just guessing at this point. I really can't see what would have changed in the last few updates that would affect the HA integration.

twa127 commented 3 years ago

@scottagecheeseandcrackers I wonder if #240 is causing the problem, if you have any sensors which have the attribute field in table mqtt_devices set to NULL these need to be changed to an empty string

scottagecheeseandcrackers commented 3 years ago

Just sent you the dump zip @twa127 . #240 had previously been an issue - but everything was running fine since the fix was pushed.

twa127 commented 3 years ago

@scottagecheeseandcrackers thanks got the database okay, had a quick look and found nothing amiss, I'll plod on but a bit difficult as I don't have HA running

JSa1987 commented 3 years ago

Unfortunately I won't have access to my computer to properly look at this until later next week. Quickly looking at the code I think this could happen in case one of the sensors (maybe linked to a zone) does not have any record in the message_in table. I think a check needs to be implemented to first check if there is any record to be processed.