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

[thing-details] Extend special handling of "actions" parameter group to all bindings #1491

Closed
mbronk opened this issue Sep 13, 2022 · 2 comments · Fixed by #1498
Closed

[thing-details] Extend special handling of "actions" parameter group to all bindings #1491

mbronk opened this issue Sep 13, 2022 · 2 comments · Fixed by #1498
Labels
enhancement New feature or request main ui Main UI

Comments

@mbronk
Copy link
Contributor

mbronk commented Sep 13, 2022

The problem

Currently, the Thing Details view handles Z-Wave binding special, and presents a list of actions (<parameter-group name="actions">) as clickable buttons, with confirmation prompt.

This behavior is nice and desirable, but specific to Z-Wave binding only:

  • // special treatment for Z-Wave actions
    if (this.thingType.UID.indexOf('zwave') === 0) {
    this.zwaveActions = this.configDescriptions.parameters.filter((p) => p.groupName === 'actions')
    this.configDescriptions.parameters = this.configDescriptions.parameters.filter((p) => p.groupName !== 'actions')
    }

  • <f7-block class="block-narrow" v-if="ready && !error && thingType && thingType.UID.indexOf('zwave') === 0">
    <f7-col>
    <f7-block-title>Z-Wave</f7-block-title>
    <f7-list>
    <f7-list-button color="blue" title="View Network Map" @click="openZWaveNetworkPopup" />
    <f7-list-button color="blue" v-for="action in zwaveActions" :key="action.name" :title="action.label" @click="doZWaveAction(action)" />
    </f7-list>
    </f7-col>
    <z-wave-network-popup :opened="zwaveNetworkPopupOpened" @closed="zwaveNetworkPopupOpened = false" />
    </f7-block>

Your suggestion

Make the abovementioned behavior more generic (remove hard-code for Z-Wave, allow other bindings to use it).

Exemplary proposal

Use the "action" behavior IFF the following heuristic match is true:
groupName == 'actions' && groupLabel === 'Actions' && groupDescription === 'Actions' && action.type === 'BOOLEAN'
^--- The values have been picked to be backwards-compatible with existing Zwave binding's implementation. Assumption is these values would never be i18n'ed and we want to avoid extra conditions.

Note: A bigger (more semantically correct) change may include changing the Configuration Description DSL to include a new parameter group and/or type (say: action) and remove the heuristic matching by group/param name. This would be a OH-core change, though and given this Z-Wave hardcode has been around for ~3y now, seems to be an overkill to introduce now IMHO.

Rationale

It is customary for some devices to include a "reset to factory default" functionality. Z-Wave binding's paradigm of showing such one-off "commands" as actions seems appropriate and would create consistent UX.

Your environment

runtimeInfo:
  version: 3.4.0

Additional information

Disclaimer: I'm currently developing a binding which would benefit from this functionality.

@mbronk mbronk added enhancement New feature or request main ui Main UI labels Sep 13, 2022
@ghys
Copy link
Member

ghys commented Sep 20, 2022

Hi @mbronk,
this makes perfect sense and would be glad to accept a PR with the implementation.
I had some concerns about "actions" groups not necessarily meant as buttons therefore it was limited to the Z-Wave binding in the original implementation (to emulate functionality from HABmin mostly), but this could certainly be extended.

mbronk added a commit to mbronk/openhab-webui that referenced this issue Sep 20, 2022
For Main UI's Thing-Details view.
Fixes openhab#1491

Signed-off-by: Mateusz Bronk <bronkm+gh@gmail.com>
Signed-off-by: mbronk <bronk.m+gh@gmail.com>
mbronk added a commit to mbronk/openhab-webui that referenced this issue Sep 20, 2022
For Main UI's Thing-Details view.
Fixes openhab#1491

Signed-off-by: Mateusz Bronk <bronk.m+gh@gmail.com>
mbronk added a commit to mbronk/openhab-webui that referenced this issue Sep 20, 2022
For Main UI's Thing-Details view.
Fixes openhab#1491

Signed-off-by: Mateusz Bronk <bronk.m+gh@gmail.com>
@mbronk
Copy link
Contributor Author

mbronk commented Sep 20, 2022

Hi @ghys,
Thx for the comment. Since this is a very subtle change, I've created a naive PR #1498 for the changes mentioned above (and added a very basic "support" (extra disclaimer/color) for verify param as well).

FYI - I do not have a fully-functional Z-Wave setup at this time, so I guess it wouldn't hurt if someone who does, also took a 2nd look as well...


Full disclosure: I actually don't even have a full on node/vue dev env, and the changes seemed small enough not to invest in setting everything up. Alas, I have not run full set of automated tests (taking a leap of faith and trusting the CI here, as far as I've seen this should be transparent to them though), and made only a rudimentary smoke test proving that the buttons do seem to work on my setup...

Sorry if this seems half-baked. I won't be able to allocate more time to it in near future and figured it is worth submitting as-is. If the PR creates more chaos than it helps - feel free to reject right off the bat.

Cheers!

mbronk added a commit to mbronk/openhab-webui that referenced this issue Sep 24, 2022
For Main UI's Thing-Details view.
Fixes openhab#1491

Signed-off-by: Mateusz Bronk <bronk.m+gh@gmail.com>
mbronk added a commit to mbronk/openhab-webui that referenced this issue Oct 1, 2022
For Main UI's Thing-Details view.
Matching is by group param context ("actions") first, then falls back to
"by name" for Z-Wave binding backwards-compatibility.
Fixes openhab#1491

Signed-off-by: Mateusz Bronk <bronk.m+gh@gmail.com>
mbronk added a commit to mbronk/openhab-webui that referenced this issue Oct 1, 2022
For Main UI's Thing-Details view.
Matching is by group param context ("actions") first, then falls back to
"by name" for Z-Wave binding backwards-compatibility.
Fixes openhab#1491

Signed-off-by: Mateusz Bronk <bronk.m+gh@gmail.com>
mbronk added a commit to mbronk/openhab-webui that referenced this issue Oct 1, 2022
For Main UI's Thing-Details view.
Matching is by group param context ("actions") first, then falls back to
"by name" for Z-Wave binding backwards-compatibility.
Fixes openhab#1491

Signed-off-by: Mateusz Bronk <bronk.m+gh@gmail.com>
mbronk added a commit to mbronk/openhab-webui that referenced this issue Oct 7, 2022
For Main UI's Thing-Details view.
Matching is by group param context ("actions") first, then falls back to
"by name" for Z-Wave binding backwards-compatibility.
Fixes openhab#1491

Signed-off-by: Mateusz Bronk <bronk.m+gh@gmail.com>
mbronk added a commit to mbronk/openhab-webui that referenced this issue Oct 7, 2022
For Main UI's Thing-Details view.
Matching is by group param context ("actions") first, then falls back to
"by name" for Z-Wave binding backwards-compatibility.
Fixes openhab#1491

Signed-off-by: Mateusz Bronk <bronk.m+gh@gmail.com>
@ghys ghys closed this as completed in #1498 Oct 8, 2022
ghys pushed a commit that referenced this issue Oct 8, 2022
For Main UI's Thing-Details view.
Matching is by group param context ("actions") first, then falls back to
"by name" for Z-Wave binding backwards-compatibility.

Closes #1491.

Signed-off-by: Mateusz Bronk <bronk.m+gh@gmail.com>
Also-by: Yannick Schaus <github@schaus.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request main ui Main UI
Projects
None yet
2 participants