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

MQTT Buttons state not updated - Home Assistant #199

Closed
fmunozs opened this issue Sep 7, 2022 · 16 comments · Fixed by #209
Closed

MQTT Buttons state not updated - Home Assistant #199

fmunozs opened this issue Sep 7, 2022 · 16 comments · Fixed by #209
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@fmunozs
Copy link
Contributor

fmunozs commented Sep 7, 2022

Hello, I've just installed the latest version and connected it with my local Home Assistant server.

I can see correctly the LED Matrix state, Illuminance sensor, however none of the 3 buttons are showing up its state. The state is being shown on the PixelIt panel correctly but not on MQTT.

image

@o0shojo0o
Copy link
Member

This might be related to the change in version 1.0.0 (2022-07-22),
but I don't really know about Home Assistant... @foorschtbar or @pplucky can have a look over here?

@o0shojo0o o0shojo0o added bug Something isn't working help wanted Extra attention is needed labels Sep 8, 2022
@meli-femunoz
Copy link

Just want do add that I also tried the 0.0.4 version and the events didnt show up either on Home Assistant.

@pplucky
Copy link
Contributor

pplucky commented Sep 8, 2022

Just want do add that I also tried the 0.0.4 version and the events didnt show up either on Home Assistant.

@fmunozs @meli-femunoz Can you please check if the values appear in the mqtt topics on the mqtt server when you click the buttons? Myself I use MQTT Explorer for this purpose, you can try it out if you have no other tool.

If yes, can you please provide the exact topic names?

@fmunozs
Copy link
Contributor Author

fmunozs commented Sep 9, 2022

hello pplucky! thanks for pointing me to MQTT Explorer. I can see the message going to the sever, however it doesnt seem to be using the "automatic" format that Home Assistant uses and that must be why its not detected. I will see if I can fix it. thanks

@pplucky
Copy link
Contributor

pplucky commented Sep 9, 2022

hello pplucky! thanks for pointing me to MQTT Explorer. I can see the message going to the sever, however it doesnt seem to be using the "automatic" format that Home Assistant uses and that must be why its not detected. I will see if I can fix it. thanks

I can also confirm that this is pre 1.0.0 behavior, as I didn't touch the button code (I had no way of testing it at the time).

As per my analysis, it seems the MQTT discovery topic is being sent from PixelIT to HA, but without any value published. If you look at the block of code within this cycle, you'll notice that unlike what happens for other sensors, no line like this one, exists for the buttons.

Maybe @o0shojo0o can help figure out where are the values to be published (the same as in the UI). so that this can be fixed.

Edit: If I saw this right, it seems that the possible values are true/false, so maybe it would even make more sense to transform these into binary sensors, rather than sensors...

@o0shojo0o
Copy link
Member

Maybe @o0shojo0o can help figure out where are the values to be published (the same as in the UI). so that this can be fixed.

In teorie, the event <mqttMasterTopic>/buttons/ should come one of these JSON {"leftButton":true} , {"middleButton":true} or {"rightButton":true}, or just with false.
Here is the line:

client.publish((mqttMasterTopic + "buttons").c_str(), String("{\"" + btnAPINames[button] + "\":" + (state ? "true" : "false") + "}").c_str(), true);

@pplucky
Copy link
Contributor

pplucky commented Sep 11, 2022

@o0shojo0o @fmunozs @meli-femunoz Is any of you willing to try my fork?

Using @o0shojo0o's tip above, I managed to fix a couple of things that didn't seem OK to me and renamed the buttons to Left, Middle and Right instead of just Button0, Button1 and Button2 (which still stands as the sensor ID).

Playing around with MQTT, I managed to get these to appear:
image

But as I don't have buttons assembled, I cannot do the real testing.

If it looks OK, I can create a PR for it.

@fmunozs
Copy link
Contributor Author

fmunozs commented Sep 11, 2022

Hello pplucky, I tried your fork but still cant see the value on HA. I see the update events on the "buttons" topic, and see the new val_tpl but it doesnt show yet. Also, I believe that the current implementation of MQTT for button state can be improved because currently its only possible to see the state of a single button. If I press 2 or 3 buttons at the same time only the last press is reflected.

image

I will try to send a PR in an hour, I also think that it is better to change the dev_cla for the sensors because it is not a timestamp.

@pplucky
Copy link
Contributor

pplucky commented Sep 11, 2022

Hello pplucky, I tried your fork but still cant see the value on HA. I see the update events on the "buttons" topic, and see the new val_tpl but it doesnt show yet. Also, I believe that the current implementation of MQTT for button state can be improved because currently its only possible to see the state of a single button. If I press 2 or 3 buttons at the same time only the last press is reflected.

Yes, that's a fact. My only purpose at this time was to make it MQTT discovery work also for buttons.

Actually, there is a single MQTT topic for all buttons which is being overwritten each time a button is pressed, but this has nothing to do with MQTT discovery, as discovery is only getting information from the existing MQTT topics to feed the discovery-generated entities.

image

I will try to send a PR in an hour, I also think that it is better to change the dev_cla for the sensors because it is not a timestamp.

Damn, I forgot to remove the dev_cla, that is why it is not working. Fixed now in my fork. I added PR to include those fixes already.

@fmunozs
Copy link
Contributor Author

fmunozs commented Sep 12, 2022

Hello! the last one shows the value now, great! I am checking your fork to try and send a new PR to allow multiple buttons.

image

@pplucky
Copy link
Contributor

pplucky commented Sep 12, 2022

Hello! the last one shows the value now, great! I am checking your fork to try and send a new PR to allow multiple buttons.

image

Untested, but tried creating a new MQTT topic under buttons named after the corresponding button (button1, button2 or button3) and use its state to feed MQTT discovery directly...

@fmunozs
Copy link
Contributor Author

fmunozs commented Sep 12, 2022

Tried to compile it but it fails.

C:/Users/fmunozs/Desktop/fork/src/PixelIt.ino: In function 'boolean MQTTreconnect()':
C:/Users/fmunozs/Desktop/fork/src/PixelIt.ino:2459:89: error: conversion from 'const __FlashStringHelper*' to 'const StringSumHelper' is ambiguous
                 payload.replace(F("#STATETOPIC#"), String(F("buttons/button") + String(n)));
                                                                                         ^

@pplucky
Copy link
Contributor

pplucky commented Sep 12, 2022

Tried to compile it but it fails.

C:/Users/fmunozs/Desktop/fork/src/PixelIt.ino: In function 'boolean MQTTreconnect()':
C:/Users/fmunozs/Desktop/fork/src/PixelIt.ino:2459:89: error: conversion from 'const __FlashStringHelper*' to 'const StringSumHelper' is ambiguous
                 payload.replace(F("#STATETOPIC#"), String(F("buttons/button") + String(n)));
                                                                                         ^

Can you please try again?

@fmunozs
Copy link
Contributor Author

fmunozs commented Sep 12, 2022

Works great! Pressed the middle and right button and both show as pressed.

image

@pplucky
Copy link
Contributor

pplucky commented Sep 12, 2022

Works great! Pressed the middle and right button and both show as pressed.

Thanks for letting me know. I think i'd rather transform these into separate binary sensors before submitting a PR.

Care for trying again? Looking good here (all included in same PR):
image

@fmunozs
Copy link
Contributor Author

fmunozs commented Sep 12, 2022

Everything seems to be working OK! I had to remove the device from HA because the old button sensors configuration appeared next to the new ones, maybe its a good idea to mention that somewhere for users upgrading from an older firmware version :)

2022-09-11.20-53-02.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants