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.

Sign up for GitHub

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] fix state/command values for switches #6589

Merged
merged 1 commit into from
Dec 19, 2019
Merged

[mqtt][homeassistant] fix state/command values for switches #6589

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions ...istant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/ComponentSwitch.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ static class ChannelConfiguration extends BaseChannelConfiguration {
protected boolean optimistic = false;

protected String state_topic = "";
protected String state_on = "true";
protected String state_off = "false";
protected String state_on = "ON";
protected String state_off = "OFF";
Copy link
Contributor

Choose a reason for hiding this comment

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

The states are optional ans should be equal to the payloads, if not present.
So they should be @Nullable and then:

        String sOn = channelConfiguration.state_on;
        if (sOn == null) {
            sOn = channelConfiguration.payload_on;
        }
        String sOff = channelConfiguration.state_off;
        if (sOff == null) {
            sOff = channelConfiguration.payload_off;
        }

        OnOffValue value = new OnOffValue(sOn, sOff, channelConfiguration.payload_on, channelConfiguration.payload_off);

Copy link
Member Author

@J-N-K J-N-K Dec 14, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Good point

Edit: since this is a static class (and cannot be altered in any way because there is no configuration for that) there is no benefit in adding that.

Copy link
Contributor

Choose a reason for hiding this comment

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

I do not get youe comment (the edit part)
The config is deserialized in the parent class from a JSON, which is read from an MQTT topic.
So the configuration is in use.
But we can for sure handle this in another PR

protected @Nullable String command_topic;
protected String payload_on = "true";
protected String payload_off = "false";
protected String payload_on = "ON";
protected String payload_off = "OFF";
}

public ComponentSwitch(CFactory.ComponentConfiguration componentConfiguration) {
Expand Down