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

[mqtt.homeassistant] Implement Vacuum discovery for Homeassistant MQTT #11216

Merged
merged 3 commits into from
Nov 7, 2021

Conversation

t2000
Copy link
Contributor

@t2000 t2000 commented Sep 7, 2021

Closes #8988

Automatically discovers vacuum robots running on Valetudo (RE) with enabled MQTT

Signed-off-by: Stefan Triller [email protected]

@t2000 t2000 requested a review from davidgraeff as a code owner September 7, 2021 15:54
@kaikreuzer kaikreuzer added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Sep 9, 2021
@Skinah Skinah added the enhancement An enhancement or new feature for an existing add-on label Sep 9, 2021
@fwolter
Copy link
Member

fwolter commented Oct 24, 2021

There's a conflict due to the last commit to mqtt.homeassistant.

@t2000
Copy link
Contributor Author

t2000 commented Nov 3, 2021

@fwolter I have rebased my changes and resolved the conflict.

*/
@NonNullByDefault
public class Vacuum extends AbstractComponent<Vacuum.ChannelConfiguration> {
public static final String vacuumStateChannelID = "state";
Copy link
Member

Choose a reason for hiding this comment

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

Constants should be all upper case.

}

protected @Nullable String command_topic;
protected String state_topic = "";
Copy link
Member

Choose a reason for hiding this comment

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

Normal members should be camelCase.

@t2000
Copy link
Contributor Author

t2000 commented Nov 7, 2021

Thanks for the review, I have fixed the format error as well and rebased against the latest main branch too.

Signed-off-by: Stefan Triller <[email protected]>
@fwolter fwolter merged commit 8dd4559 into openhab:main Nov 7, 2021
@fwolter fwolter added this to the 3.2 milestone Nov 7, 2021
@t2000 t2000 deleted the mqttVacuum branch November 7, 2021 15:41
}

protected @Nullable String commandTopic;
protected String stateTopic = "";
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this work?
Don't we need a @SerializedName("state_topic") vor the deserialization to work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, yes!

Thank you very much. I have overlooked this and just blindly integrated the review comments as I knew they made sense in a way that there are styles on how to name variables.

I have created a small PR to fix this: #11550

dschoepel pushed a commit to dschoepel/openhab-addons that referenced this pull request Nov 9, 2021
openhab#11216)

* Implement Vacuum discovery for Homeassistant MQTT

Closes openhab#8988

Signed-off-by: Stefan Triller <[email protected]>

* Addressed review comments

Signed-off-by: Stefan Triller <[email protected]>

* Spotless run again

Signed-off-by: Stefan Triller <[email protected]>
Signed-off-by: Dave J Schoepel <[email protected]>
@antroids
Copy link
Contributor

antroids commented Nov 9, 2021

Closes #8988

Automatically discovers vacuum robots running on Valetudo (RE) with enabled MQTT

Signed-off-by: Stefan Triller [email protected]

Hi,
thank you for your contribution!

Could you please show where you found specification of status attributes, like time or usage counters?
I'm not 100% sure, but looks like these attributes were removed in latest Valetudo versions and they still exists only for Homie. Unfortunately, I'm far away from home and can't check this.
There is also lack of full support of Home Assistant mqtt.vacuum specification in your implementation.

I have a branch with mqtt.vacuum specification fully implemented: https://github.com/antroids/openhab-addons/tree/mqtt_vacuum
We could merge status attributes into the branch, if there is specification on them somewhere.

@t2000
Copy link
Contributor Author

t2000 commented Nov 9, 2021

Could you please show where you found specification of status attributes, like time or usage counters?

I have used https://www.home-assistant.io/integrations/vacuum.mqtt/ and he output of my valetudo running vacuum cleaners :)

I'm not 100% sure, but looks like these attributes were removed in latest Valetudo versions and they still exists only for Homie. Unfortunately, I'm far away from home and can't check this.

You are right! Just when I had finished my implementation and when I was searching for possibilities to overcome the map rotation issues in the robots, I also saw that valetudo has changed the mqtt schema to homie. But that shouldn't be an issue, since we could also implement this too.

There is also lack of full support of Home Assistant mqtt.vacuum specification in your implementation.
my implementation was never meant to cover 100% of any spec. I built it to support the robots out of the box with all the features I need. Feel free to add more, it is OSS.

I have a branch with mqtt.vacuum specification fully implemented: https://github.com/antroids/openhab-addons/tree/mqtt_vacuum
We could merge status attributes into the branch, if there is specification on them somewhere.

I didn't know that. How can one know this if you do not leave a comment on the issue that I have closed? and why did you not create a PR yourself earlier or have commented on mine?

Anyway, feel free to add your stuff from your branch, which is missing in my implementation. I mean, in the end this is how OSS works, right? :)

@antroids
Copy link
Contributor

antroids commented Nov 9, 2021

Maybe you have a mqtt messages from new Valetudo version?
I would like to figure out how to handle these schema extensions, because they are specific for particular firmware version.
Maybe enable them based on manufacturer and topic prefix.

jpg0 pushed a commit to jpg0/openhab-addons that referenced this pull request Nov 10, 2021
openhab#11216)

* Implement Vacuum discovery for Homeassistant MQTT

Closes openhab#8988

Signed-off-by: Stefan Triller <[email protected]>

* Addressed review comments

Signed-off-by: Stefan Triller <[email protected]>

* Spotless run again

Signed-off-by: Stefan Triller <[email protected]>
NickWaterton pushed a commit to NickWaterton/openhab-addons that referenced this pull request Dec 30, 2021
openhab#11216)

* Implement Vacuum discovery for Homeassistant MQTT

Closes openhab#8988

Signed-off-by: Stefan Triller <[email protected]>

* Addressed review comments

Signed-off-by: Stefan Triller <[email protected]>

* Spotless run again

Signed-off-by: Stefan Triller <[email protected]>
Signed-off-by: Nick Waterton <[email protected]>
mischmidt83 pushed a commit to mischmidt83/openhab-addons that referenced this pull request Jan 9, 2022
openhab#11216)

* Implement Vacuum discovery for Homeassistant MQTT

Closes openhab#8988

Signed-off-by: Stefan Triller <[email protected]>

* Addressed review comments

Signed-off-by: Stefan Triller <[email protected]>

* Spotless run again

Signed-off-by: Stefan Triller <[email protected]>
Signed-off-by: Michael Schmidt <[email protected]>
nemerdaud pushed a commit to nemerdaud/openhab-addons that referenced this pull request Jan 28, 2022
openhab#11216)

* Implement Vacuum discovery for Homeassistant MQTT

Closes openhab#8988

Signed-off-by: Stefan Triller <[email protected]>

* Addressed review comments

Signed-off-by: Stefan Triller <[email protected]>

* Spotless run again

Signed-off-by: Stefan Triller <[email protected]>
marcfischerboschio pushed a commit to bosch-io/openhab-addons that referenced this pull request May 5, 2022
openhab#11216)

* Implement Vacuum discovery for Homeassistant MQTT

Closes openhab#8988

Signed-off-by: Stefan Triller <[email protected]>

* Addressed review comments

Signed-off-by: Stefan Triller <[email protected]>

* Spotless run again

Signed-off-by: Stefan Triller <[email protected]>
andan67 pushed a commit to andan67/openhab-addons that referenced this pull request Nov 6, 2022
openhab#11216)

* Implement Vacuum discovery for Homeassistant MQTT

Closes openhab#8988

Signed-off-by: Stefan Triller <[email protected]>

* Addressed review comments

Signed-off-by: Stefan Triller <[email protected]>

* Spotless run again

Signed-off-by: Stefan Triller <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[mqtt] Support for Home Assistant Component: Vacuum
6 participants