Travis CI build for [org.openhab.binding.mqtt-homeassistant]

Hello,

I’ve been trying to include a couple of small bugfixes in mqtt-homeassistant, but I get tons of esoteric errors from the Travis CI build. Would appreciate if someone who knows how this works could take a look. @David_Graeff, @jochen314.

I’ve submitted pull request #7025 https://github.com/openhab/openhab-addons/pull/7025. I think there are some useful bugfixes there, but I’m concerned that they won’t get reviewed if the CI build keeps failing. Without any additional knowledge regarding how the Travis builds were configured, debugging that is a bit more of a morass than I wanted to get into right now.

Thanks,

-Matt Hoekstra

The build errors don’t appear related to the changes you made.
However, I believe your logic change is incorrect.

@namraccr thanks for the feedback.

I’ll admit that I don’t fully follow the deign of the recent changes that add availability updates to Generic MQTT things, although that’s a great idea, that’s why I switched to MQTT Homeassistant in the first place a week ago, to get availability updates.

Without my logic change, I see in the debugger that messageReceived is set to true after the first update on the availability topic from the MQTT server. Then after that, you can see from the logic that the thing will always be set to ThingStatus.ONLINE, regardless of the value of availabilityTopicsSeen. In the case of an MQTT Homeassistant compliant light, changing the payload of the availability topic from online to offline causes the value of “availabilityTopicsSeen” to toggle between true and false. But since messageReceived is always true after receiving the first update the Thing is always set to ThingStatus.ONLINE. With my update, the Thing is online until receiving the first availability update message, I was guessing that this was the intention in the case where no availability_topic was registered. But once an availability message is received, the value of availabilityTopicsSeen controls whether the Thing is set online or offline. In the case of my light, with this change to the logic, the Thing exhibits the expected behavior. When I set the payload of the availability_topic to “online” the Thing shows online, and when I set the payload to “offline” the Thing shows offline in OpenHAB.

Upon further reflection, perhaps I got the intention backward, and instead I should put the ! in front of availabilityTopicsSeen. I may have been incorrect above about availabilityTopicsSeen toggling. I’ll check again.

I was correct. The parameter names are perhaps confusing, but it is availabilityTopicsSeen that toggles based on the MQTT value. Here is the code that calls the function updateThingStatus, from line 365 in AbstractMQTTThingHandler. You can see that the value of availabilityTopicsSeen comes from reading the availability values from the MQTT broker.

    if (availabilityStates.isEmpty()) {
        availabilityTopicsSeen = true;
    } else {
        availabilityTopicsSeen = availabilityStates.values().stream().allMatch(
                c -> c != null && OnOffType.ON.equals(c.getCache().getChannelState().as(OnOffType.class)));
    }
    updateThingStatus(messageReceived.get(), availabilityTopicsSeen);
if (availabilityStates.isEmpty()) {
        availabilityTopicsSeen = true;
} 

Doesn’t this then seem like the real error? If the availabilityStates map is empty, then the boolean should be FALSE, not TRUE.

I suppose it’s best for @jochen314 to weigh in, since this is his design. But the way I read it, he’s setting the value to true if no availability topics are associated with the MQTT device. So if the device didn’t set an availability topic, it will always appear online, which is the expected behavior in Homeassistant.

I don’t fully understand what messageReceived.get() does. If this is checking to see if any messages have been received on this topic then I think my new logic is right. If instead it is checking to see if the connection is still alive to the MQTT broker, then I think the logic I changed should instead be (messageReceived && availabilityTopicsSeen).

Just commenting on the errors: that‘s a known issue with the integration tests. You can ignore them.

OK, thanks!

This topic was automatically closed 41 days after the last reply. New replies are no longer allowed.