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

MQTT relay payload ON / OFF inconsistent usage #1883

Closed
mcspr opened this issue Aug 30, 2019 · 0 comments · Fixed by #1885
Closed

MQTT relay payload ON / OFF inconsistent usage #1883

mcspr opened this issue Aug 30, 2019 · 0 comments · Fixed by #1885
Labels
enhancement New feature or request mqtt

Comments

@mcspr
Copy link
Collaborator

mcspr commented Aug 30, 2019

Bug description
Configuration options for payload ON and OFF strings are independent of each other. For example, if Home Assistant configuration uses payload ON as "1", relay payload needs to be updated too. Otherwise, configuration generator would show a different value from the one that is actually sent.

Steps to reproduce
Update HOMEASSISTANT_PAYLOAD_ON
Run ha.config in terminal, see payload_on
Try to subscribe to the relay topic and toggle the relay. Actual value is from RELAY_MQTT_ON

Expected behavior
Dependant MQTT messages should have a single configuration entry
Perhaps, there should be a global setting for every "boolean" payload.

Tools used

  • IDE & version Platformio, Arduino IDE with custom.h / modified .h
  • Compiler & version (if not embedded in IDE) Any

Additional context
As mentioned in the #1762 (comment), if HA config generator omits payload_on/off, Home Assistant will expect default ON and OFF strings. However, these are used:
https://github.com/xoseperez/espurna/blob/dev/code/espurna/config/general.h#L1280-L1294
But they need to be in sync with these:

espurna/code/espurna/config/general.h

Lines 407 to 413 in 35ce687

// Configure the MQTT payload for ON/OFF
#ifndef RELAY_MQTT_ON
#define RELAY_MQTT_ON "1"
#endif
#ifndef RELAY_MQTT_OFF
#define RELAY_MQTT_OFF "0"
#endif

Because that's what is actually used by mqtt send in relay module:

espurna/code/espurna/relay.ino

Line 879 in 35ce687

mqttSend(MQTT_TOPIC_RELAY, id, _relays[id].current_status ? RELAY_MQTT_ON : RELAY_MQTT_OFF);

So there should also be an option to optionally omit certain key-value pairs in the configuration.
@mcspr mcspr added enhancement New feature or request mqtt labels Aug 30, 2019
This was referenced Aug 30, 2019
Closed
Merged
mcspr added a commit that referenced this issue Sep 3, 2019
@mcspr
Home Assistant updates (#1885)
13c1a19 
- remove "platform" key, see #1440. this implicitly sets schema to "basic". pending some other clean-up regarding json and mqtt queueing, other schema can be added down the line 
- updated ws module queue elem to capture callbacks list, allows to pass more than one callback (for example, when they are generated on the fly as lambdas, see ha wsPost usage)
- modified method to send ha config to use global ws queue, fix #1762 problem with empty topics and ensure json allocation is consistent.
- use existing defines to set mqtt payload options. amend #1085, #1188, #1883 to use the set payload value. drop HOMEASSISTANT_PAYLOAD... defines. 
- update MQTT_STATUS_ONLINE/OFFLINE and RELAY_MQTT_ON/OFF with runtime configuration
- filter payload strings so that the resulting yaml value is not interpreted as bool (python True, False)
- helper method for settings to streamline string values manipulation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request mqtt
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Home Assistant updates mcspr/espurna
1 participant
@mcspr