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.

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

Boolean format API/MQTT options 1/0 and true/false don't work as expected #931

Closed
pswid opened this issue Jan 26, 2023 · 11 comments
Closed
Labels
enhancement New feature or request
Milestone

Comments

@pswid
Copy link
Contributor

pswid commented Jan 26, 2023

Latest v3.5.0-dev.16.

If I choose the 1/0 option as the Boolean Format API/MQTT, the quotation marks are added anyway in MQTT, so the option works the same as the option "1"/"0". The same problem with true/false option.

@pswid pswid added the bug Something isn't working label Jan 26, 2023
@MichaelDvP
Copy link
Contributor

MichaelDvP commented Jan 26, 2023

Only if you are using HA. That's another stupid limitation of HA. Disable HA and you get the native format.
Maybe we should skip these option from the selectbox if HA is enabled?
But HA affects only mqtt, the option is for mqtt and api. If we skip it, there is no way to use native bool in api when ha is enabled.
See here:

EMS-ESP32/src/emsdevice.cpp

Lines 1528 to 1530 in ede8e7d

if (Mqtt::ha_enabled() && (output_target == OUTPUT_TARGET::MQTT)) {
char s[12];
json[name] = Helpers::render_boolean(s, value_b); // for HA always render as string

@pswid
Copy link
Contributor Author

pswid commented Jan 26, 2023

Can you write something more about this HA limitation? Maybe there's a way to workaround it...

@MichaelDvP
Copy link
Contributor

No, i'm not using HA. Proddy has spend a lot of time and work to add code and comments for the HA extras. I'm only reading the comments (as quoted).

@proddy
Copy link
Contributor

proddy commented Jan 26, 2023

It's a limitation of HA's MQTT Discovery (e.g. https://www.home-assistant.io/integrations/switch.mqtt/) which requires payload_on and payload_off as being Strings in the MQTT payload. I'll experiment a few workarounds.

..but if you're using Home Assistant you shouldn't really care what the API/MQTT format is? Do you have a use-case @pswid that I've overlooked? Like do you need numeric 1 and 0?

@proddy proddy added question Question about something and removed bug Something isn't working labels Jan 26, 2023
@pswid
Copy link
Contributor Author

pswid commented Jan 26, 2023

in fact payload_on and payload_off doesn't need to be a string. In case of "on/off" option selected, just change in MQTT discovery config, from:

  "payload_on": "true",
  "payload_off": "false",

to:

  "payload_on": true,
  "payload_off": false,

remove the condition:

EMS-ESP32/src/emsdevice.cpp

Lines 1528 to 1530 in ede8e7d

if (Mqtt::ha_enabled() && (output_target == OUTPUT_TARGET::MQTT)) {
char s[12];
json[name] = Helpers::render_boolean(s, value_b); // for HA always render as string

and boolean option "on/off" will start working (I've just tested it). Option "0/1" too! Howewer it would also be good to remove quotes from payload_on/off definitions (just for more "clarity").

... I'm using API/MQTT not only with HA.

@proddy
Copy link
Contributor

proddy commented Jan 26, 2023

ok great, that's what I was hoping for. Home Assistant finally fixed it (I promise it didn't use to be like this!)

@proddy proddy added this to the v3.5.0 milestone Jan 26, 2023
@proddy proddy added enhancement New feature or request and removed question Question about something labels Jan 26, 2023
@pswid
Copy link
Contributor Author

pswid commented Jan 27, 2023

@proddy do you want me to try to prepare a fix?

@proddy
Copy link
Contributor

proddy commented Jan 28, 2023

If you have time that would be great. Do test all scenarios.

@pswid
Copy link
Contributor Author

pswid commented Jan 28, 2023

ok I will. hopefully this weekend

pswid added a commit to pswid/EMS-ESP32 that referenced this issue Jan 29, 2023
@pswid
Copy link
Contributor Author

pswid commented Jan 29, 2023

done. everything seems to work fine, though if you select option true/false, HA sends True/False (uppercase first letter) instead. This is not a problem, because Helpers::value2bool is case-insensitive (executes toLower(value) at the beginning).

@proddy
Copy link
Contributor

proddy commented Jan 29, 2023

Nice! I'll try it out.

proddy added a commit that referenced this issue Jan 30, 2023
fixed 1/0 and true/false MQTT/API format options with HA enabled (#931)
@proddy proddy closed this as completed Jan 30, 2023
pswid added a commit to pswid/EMS-ESP32 that referenced this issue Jan 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants