-
Notifications
You must be signed in to change notification settings - Fork 782
[mqtt] Rollershutter STOP command, outgoing transform, chained incoming transformations #6695
[mqtt] Rollershutter STOP command, outgoing transform, chained incoming transformations #6695
Conversation
6739a16
to
f461364
Compare
return; | ||
} | ||
state = newState; | ||
throw new IllegalStateException("Cannot call update() with UpDownType"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? UpDownType implements State, so updates are valid (and correspond to 0 and 100).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So they are not meant to be move commands like "move up", "move down" but more like states meaning "completely up" and "completely down"?
A user wrote me a pm saying that stop doesn't work for him (which is absolutely correct) and that up/down sends his rollershutters up/down (which is correct so far) but also updates the state to 0% and 100% which he was not expecting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the commands UP/DOWN mean that it should move to UP (=0) or DOWN (=100) position.
also updates the state to 0% and 100%
The binding itself should indeed not update the state, but by default the "autoupdate" feature of the framework does it - if it sees an UP event, it will do a state auto-update to 0%.
Note that since #5011, the binding can define an auto-update policy, so you can actually configure channels in a way that the framework suppresses the auto-update behavior. This is probably what should usually be done for rollershutters as the state is reached much later than the command has been received, so the state update should better come from the physical device.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@davidgraeff What's the status here now? Are you waiting for anything from my side?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have changed the code already and was waiting for another review. Have no time for the rebase right now though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cannot see any code change, this is why I was asking. The line that I commented still throws an IllegalStateException
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok, must have overlooked that line. When I do the rebase, I'll look into it again :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any news here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added some changes
96de2c6
to
ea345ef
Compare
This pull request has been mentioned on openHAB Community. There might be relevant details there: |
ea345ef
to
0f22534
Compare
This pull request has been mentioned on openHAB Community. There might be relevant details there: |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/mqtt2-in-openhab-2-4-rollershutter-map/61311/5 |
1 similar comment
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/mqtt2-in-openhab-2-4-rollershutter-map/61311/5 |
0f22534
to
0c153b3
Compare
@davidgraeff Could you please check the compilation failures on Travis? |
Yes of course.
|
@davidgraeff I "saw" them myself, my request was rather to fix them... |
I'm sure you saw them. That was my way of avoiding to lookup travis again for a later fix. |
This pull request has been mentioned on openHAB Community. There might be relevant details there: |
The Rollershutter channel stores its value as percentage. UP/DOWN/STOP commands received via MQTT do not change that state. Instead of trying to handle those commands, they are now posted to the framework. * Allow outgoing transformations. * Allow chained incoming transformations. * Add more examples to the readme. Signed-off-by: David Gräff <david.graeff@web.de>
…ndling completely. Readme: Add retain and isCommand and some more mqtt1 details. Rollershutter: Fix Signed-off-by: David Graeff <david.graeff@web.de>
4e12a8b
to
561fe5b
Compare
@kaikreuzer All tests fixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/mqtt-2-chained-transformations/84861/4 |
The Rollershutter channel stores its value as percentage.
UP/DOWN/STOP commands received via MQTT do not change that state.
Instead of trying to handle those commands, they are now
posted to the framework.
Signed-off-by: David Gräff david.graeff@web.de