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

[homematic] Fix min/max values for rollershutters #13821

Merged
merged 2 commits into from
Dec 3, 2022

Conversation

maniac103
Copy link
Contributor

For dimmers, the 1.0 max value sent by CCU was already converted to percent values in the item state description. Do the same thing also for roller shutters.

Previously, the item state description looked e.g. like this:

{
    "link": "http://server:8080/rest/items/BathroomBlindsLevel",
    "state": "100",
    "stateDescription": {
        "minimum": 0.0,
        "maximum": 1.0,
        "pattern": "%d %%",
        "readOnly": false,
        "options": [

        ]
    },
    "editable": true,
    "type": "Rollershutter",
    "name": "BathroomBlindsLevel",
    "label": "Level",
    "category": "Blinds",
    "tags": [
        "Point"
    ],
    "groupNames": [
    ]
}

... which is obviously wrong. Both ITEM_DIMMER and ITEM_ROLLERSHUTTER use PercentTypeConverter when converting item commands to CCU values, which is dividing the value by 100, so in both cases the maximum value in the state description should look the same.

@maniac103 maniac103 requested a review from gerrieg as a code owner December 2, 2022 10:54
@maniac103
Copy link
Contributor Author

@MHerbst Mind having a look at this?

For dimmers, the 1.0 max value sent by CCU was already converted to
percent values in the item state description. Do the same thing also for
roller shutters.

Signed-off-by: Danny Baumann <[email protected]>
@maniac103 maniac103 force-pushed the homematic-metadata-fix branch 3 times, most recently from ac59bf2 to 3e16bea Compare December 2, 2022 12:43
Signed-off-by: Danny Baumann <[email protected]>
@maniac103 maniac103 force-pushed the homematic-metadata-fix branch from 3e16bea to d7d6042 Compare December 2, 2022 13:47
@MHerbst
Copy link
Contributor

MHerbst commented Dec 2, 2022

@maniac103 I know from the past of different problems with blinds and dimmers. I could never test with these devices myself, and therefore I can't judge whether any of your changes may have a negative side effect.
But in general, the changes look OK to me. Would be good if they would be tested with real devices.

@maniac103
Copy link
Contributor Author

maniac103 commented Dec 2, 2022

I just remembered I also have a dimmer lying around, will do another test with that one tomorrow. No HMIP devices here though...
Would be great to get this fix into 3.4, since it prevents using Android device controls for HM devices at the moment, because the Android app uses the min/max values from the state description to tell Android about the possible selection range.

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
Copy link
Contributor

lolodomo commented Dec 3, 2022

@maniac103 : shall I wait for you test before merging ?

@lolodomo lolodomo added the bug An unexpected problem or unintended behavior of an add-on label Dec 3, 2022
@maniac103
Copy link
Contributor Author

@lolodomo I think it should be fine to merge. Just tested state description output with a roller shutter:

{
    "link": "http://127.0.0.1:8080/rest/items/MEQ0391932_Level",
    "state": "0",
    "stateDescription": {
        "minimum": 0,
        "maximum": 100,
        "pattern": "%d %%",
        "readOnly": false,
        "options": [

        ]
    },
    "editable": true,
    "type": "Rollershutter",
    "name": "MEQ0391932_Level",
    "label": "Level",
    "category": "Blinds",
    "tags": [
        "Point"
    ],
    "groupNames": [

    ]
}

and a dimmer:

{
    "link": "http://127.0.0.1:8080/rest/items/MEQ1118997_Level",
    "state": "0",
    "stateDescription": {
        "minimum": 0,
        "maximum": 100,
        "pattern": "%d %%",
        "readOnly": false,
        "options": [

        ]
    },
    "editable": true,
    "type": "Dimmer",
    "name": "MEQ1118997_Level",
    "label": "Level",
    "category": "",
    "tags": [
        "Point"
    ],
    "groupNames": [

    ]
}

and output looks as expected in both cases now.

@lolodomo lolodomo merged commit 86e3b65 into openhab:main Dec 3, 2022
@lolodomo lolodomo added this to the 3.4 milestone Dec 3, 2022
morph166955 pushed a commit to morph166955/openhab-addons that referenced this pull request Dec 18, 2022
* [homematic] Fix min/max values for rollershutters

For dimmers, the 1.0 max value sent by CCU was already converted to
percent values in the item state description. Do the same thing also for
roller shutters.

Signed-off-by: Danny Baumann <[email protected]>
Signed-off-by: Ben Rosenblum <[email protected]>
andrasU pushed a commit to andrasU/openhab-addons that referenced this pull request Dec 24, 2022
* [homematic] Fix min/max values for rollershutters

For dimmers, the 1.0 max value sent by CCU was already converted to
percent values in the item state description. Do the same thing also for
roller shutters.

Signed-off-by: Danny Baumann <[email protected]>
Signed-off-by: Andras Uhrin <[email protected]>
borazslo pushed a commit to borazslo/openhab-mideaac-addon that referenced this pull request Jan 8, 2023
* [homematic] Fix min/max values for rollershutters

For dimmers, the 1.0 max value sent by CCU was already converted to
percent values in the item state description. Do the same thing also for
roller shutters.

Signed-off-by: Danny Baumann <[email protected]>
psmedley pushed a commit to psmedley/openhab-addons that referenced this pull request Feb 23, 2023
* [homematic] Fix min/max values for rollershutters

For dimmers, the 1.0 max value sent by CCU was already converted to
percent values in the item state description. Do the same thing also for
roller shutters.

Signed-off-by: Danny Baumann <[email protected]>
nemerdaud pushed a commit to nemerdaud/openhab-addons that referenced this pull request Feb 28, 2023
* [homematic] Fix min/max values for rollershutters

For dimmers, the 1.0 max value sent by CCU was already converted to
percent values in the item state description. Do the same thing also for
roller shutters.

Signed-off-by: Danny Baumann <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem or unintended behavior of an add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants