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.homeassistant] Add support for Button component #15892

Merged
merged 2 commits into from
Nov 19, 2023

Conversation

ccutrer
Copy link
Contributor

@ccutrer ccutrer commented Nov 13, 2023

No description provided.

Signed-off-by: Cody Cutrer <cody@cutrer.us>
@ccutrer ccutrer requested review from antroids and a team as code owners November 13, 2023 22:07
@antroids
Copy link
Contributor

Probably org.openhab.binding.mqtt.generic.values.TextValue is a better option for stateless channel with a single command.

@ccutrer
Copy link
Contributor Author

ccutrer commented Nov 14, 2023

I don't have a strong preference either way. There is definitely an impact to it. For the following items:

Switch NorthGate_Restart "Restart North Gate" { channel="mqtt:homeassistant_083af268005c:mosquitto:083af268005c:083af268005c_2Dbutton_2Dba0e8e32#button" }
String HeatPUmp_ClearErrors "Clear Errors" { channel="mqtt:homie300:mosquitto:aurora-180501914:faults#clear_2Dhistory" }

Both of them have a command description reflecting a single viable choice. The former is a Switch from a Home Assistant Button, with this PR. The latter is a String, coming from a Homie property of Enum type with a single option.

With the following sitemap:

sitemap test label="Test" {
  Switch item=NorthGate_Restart
  Switch item=HeatPUmp_ClearErrors
}

They show up in the iOS app like this:
IMG_4845

And in a Web Browser like this:
Screenshot 2023-11-14 at 7 16 12 AM

If I change the widget type to Default, they show up the same in both places. I'm a little surprised the iOS app doesn't do better. My ideal for both would be a single push button that's already labeled with no extra work.

From MainUI, when going to look at the Item, they look like this:

Screenshot 2023-11-14 at 7 24 11 AM
Screenshot 2023-11-14 at 7 25 22 AM

Notice that the Switch item is immediately operable, but the String item is not. I don't use pages built from MainUI, so can't compare a basic usage from there. So at this point, a String item is slightly preferred when using BasicUI in a browser, and Switch item is slightly preferred for viewing the item directly in MainUI. My one pushback to a string item is that when using it from rules, you have to actually go figure out what value it is you're supposed to send, instead of implicitly knowing it's ON. I do agree that semantically a String item makes more sense, since there's no expectation that its possible states/commands will be ON and OFF (the latter not making sense), whereas a String you're aware it could be anything, so aren't surprised with it being limited to a single value. One thing I need to work on (and plan to do as a separate PR) is to set an auto update policy of VETO so that the item doesn't actually change states, and UIs won't reflect it's in that state. Buttons are intended to be able to be pressed multiple times, and are stateless. And regardless of all this, it's possible for the UIs to improve and better handle when an item has a command description with a single option, as well as MainUI displaying command descriptions. So I lean towards your argument that a String item is better, since its a better fit for the model semantically.

@jlaur, @lolodomo: do you guys want to weigh in?

@lolodomo lolodomo added the enhancement An enhancement or new feature for an existing add-on label Nov 18, 2023
Signed-off-by: Cody Cutrer <cody@cutrer.us>
Copy link
Contributor

@antroids antroids left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!
Now we have the both versions 👍

Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

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

LGTM, thank you

@lolodomo lolodomo merged commit fc95794 into openhab:main Nov 19, 2023
3 checks passed
@lolodomo lolodomo added this to the 4.1 milestone Nov 19, 2023
@ccutrer ccutrer deleted the mqtt-homeassistant-button branch November 19, 2023 16:35
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Nov 26, 2023
* [mqtt.homeassistant] Add support for Button component
* use a StringValue instead of an OnOffValue

---------

Signed-off-by: Cody Cutrer <cody@cutrer.us>
austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
* [mqtt.homeassistant] Add support for Button component
* use a StringValue instead of an OnOffValue

---------

Signed-off-by: Cody Cutrer <cody@cutrer.us>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants