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.

Sign up for GitHub

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

Jump to bottom

Improved Homeassistant MQTT Discovery #260

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Improved Homeassistant MQTT Discovery #260

wants to merge 1 commit into from

Conversation

javimurcia
Copy link

  • Improved Homeassistant MQTT discovery: all ESPAltherma's entities should apear under the same device.
  • Make the thermostat relay (switch) optional, allowing users to disable it (by not defining its pin) so it doesn't show up on home assistant.
  • Added the option to change the active level for the thermostat relay

Improved Homeassistant MQTT discovery so all ESPAltherma's entities a…
dc6d286 
…ppear under the same device.

Make the thermostat relay (switch) optional, allowing users to disable it (by not defining its pin) so it doesn't show up on home assistant.

Added the option to change the active level for the thermostat relay
@javimurcia javimurcia changed the title Improved Homeassistant MQTT Improved Homeassistant MQTT Discovery Mar 7, 2023
raomin
raomin reviewed Mar 12, 2023
View reviewed changes
Copy link
Owner

@raomin raomin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two small comments for your consideration. But great improvement overall, thanks.
I will test it to see if it breaks stuff on mine, and merge afterwards.

src/main.cpp

#ifdef PIN_THERM
// Thermostat relay - Set first to the inactive state, before configuring as outputs (avoid false triggering when initializing)
digitalWrite(PIN_THERM, THERM_RELAY_INACTIVE_STATE);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one should come after pinMode

include/mqtt.h
// Subscribe
// Thermostat relay
#ifdef PIN_THERM
client.publish("homeassistant/switch/espAltherma/switch/config", "{\"availability\":[{\"topic\":\"espaltherma/LWT\",\"payload_available\":\"Online\",\"payload_not_available\":\"Offline\"}],\"availability_mode\":\"all\",\"unique_id\":\"espaltherma_switch\",\"device\":{\"identifiers\":[\"ESPAltherma\"],\"manufacturer\":\"ESPAltherma\",\"model\":\"M5StickC PLUS ESP32-PICO\",\"name\":\"ESPAltherma\"},\"icon\":\"mdi:water-boiler\",\"name\":\"EspAltherma Heat Pump Demand\",\"command_topic\":\"espaltherma/POWER\",\"state_topic\":\"espaltherma/STATE\",\"payload_on\":\"ON\",\"payload_off\":\"OFF\"}", true);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Model depends on the esp32 used. I would remove it, or put a more generic "ESP"

@ap20132022
Copy link

I try to build your version, but I got an error:
include/mqtt.h:134:29: error: 'THERM_RELAY_ACTIVE_STATE' was not declared in this scope

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raomin raomin raomin left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@javimurcia @ap20132022 @raomin