[ecovacs] Initial contribution

[maniac103]

Add initial version of a binding for vacuum cleaners made by Ecovacs. It should work for all devices made in the last 3 years, but
only a limited subset of those have been tested actively (the README spells out the respective devices).

Release for testing in the marketplace: here

[maniac103]

@Flowdalic l would appreciate some help regarding the integration of Smack here. My problem is the following:

• I only need smack-core for this binding, as I don't need any actual chat features, and Ecovacs' XMPP server doesn't support them anyway
• The XMPP client binding, on the other side, does need IM features
• Since smack-core is loaded as OSGi bundle, there's only one instance of both code and static variables for both
• Smack's initializer code will try to load any relevant classes in the classpath, which will automagically enable any feature in the class path for all connections by a callback system in static variables
• This means that (at least) when the XMPP binding is loaded, the IM features (e.g. roster) are active for my connections as well
• Ecovacs' server does not support roster, so each connection attempt yields error messages and stack traces in the log
• Unfortunately, the XMPP connection to the vacuum seems a bit unstable, so I have to reconnect rather often, which leads to a lot of clutter in the log

I now need some suggestions how to proceed from here. Could I somehow disable the extended features just for a single connection (I tried, but didn't find a way)? Can the error message on roster reload failure be toned down in case the server doesn't support it? (I would have submitted a PR for that, but I seemingly can't build Smack on my machine; probably due to wrong Gradle version?) Is there another way to split the use cases of the XMPP and Ecovacs bindings?
I'd appreciate any pointers. Thanks!

[maniac103]

The WIP tag is only for the XMPP issue described above, everything else in the binding should be ready for review.

[Flowdalic]

• This means that (at least) when the XMPP binding is loaded, the IM features (e.g. roster) are active for my connections as well

Have you tried Roster.setLoadedOnLogin(false)?

Can the error message on roster reload failure be toned down in case the server doesn't support it?

IIRC the XMPP roster is a non discoverable feature. But the IQ request that yield an IQ error response that indicates that the service does not support the roster, which we may could use to change the logging behavior.

I would have submitted a PR for that, but I seemingly can't build Smack on my machine; probably due to wrong Gradle version?

Smack currently needs Gradle 6.8.3 to build.

Is there another way to split the use cases of the XMPP and Ecovacs bindings?

Ideally, and if I understood your issue(s) correctly, we wouldn't need that.

[Flowdalic]

I had a quick look at your implementation and left some comments on things that looked strange to me. I hope you do not mind, especially since I may not fully understood what's done here.

[maniac103]

Have you tried Roster.setLoadedOnLogin(false)?

I can not find that method in Smack's sources?

But the IQ request that yield an IQ error response that indicates that the service does not support the roster, which we may could use to change the logging behavior.

It does:

2022-02-03 07:43:50.151 [TRACE] [.internal.api.impl.EcovacsXmppDevice] - XXX: SENT (0): <iq id='GWpLy-10' type='get'><query xmlns='jabber:iq:roster'></query></iq>
2022-02-03 07:43:50.161 [TRACE] [.internal.api.impl.EcovacsXmppDevice] - XXX: RECV (0): <iq type="error" to="[email protected]/e7a4f07a" from="ecouser.net" id="GWpLy-10"><error type="cancel" code="501"><feature-not-implemented xmlns="urn:ietf:params:xml:ns:xmpp-stanzas"/></error></iq>
2022-02-03 07:43:50.180 [ERROR] [org.jivesoftware.smack.roster.Roster] - Exception reloading roster
org.jivesoftware.smack.XMPPException$XMPPErrorException: XMPP error reply received from ecouser.net: XMPPError: feature-not-implemented - cancel
	at org.jivesoftware.smack.XMPPException$XMPPErrorException.ifHasErrorThenThrow(XMPPException.java:185) ~[?:?]
	at org.jivesoftware.smack.XMPPException$XMPPErrorException.ifHasErrorThenThrow(XMPPException.java:179) ~[?:?]
	at org.jivesoftware.smack.AbstractXMPPConnection$7.processStanza(AbstractXMPPConnection.java:1542) ~[?:?]
	at org.jivesoftware.smack.AbstractXMPPConnection$5.run(AbstractXMPPConnection.java:1223) ~[?:?]
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) [?:?]
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) [?:?]
	at java.lang.Thread.run(Thread.java:829) [?:?]

... and that was my idea for the logging change: reduce the logging level if the error indicates the feature is not supported.

Smack currently needs Gradle 6.8.3 to build.

That one works, thank you. I'll submit a PR for the logging change. (Edit: PR done)

And thank you for the prompt and detailed responses 👍

[Flowdalic]

Have you tried Roster.setLoadedOnLogin(false)?

I can not find that method in Smack's sources?

Sorry it is Roster.setRosterLoadedAtLogin().

[maniac103]

Sorry it is Roster.setRosterLoadedAtLogin().

That one works and fixes the log spam, thanks!

[maniac103]

@openhab/add-ons-maintainers May I ask for a review and/or help with the OSGi question, please? Thanks.

[jlaur]

Draft status will effectively prevent the merge button from being accessible, while a lot of text conversations can be harder to interpret and ultimately could lead to a wrong decision being made.

I lost track. I still see the "help wanted" label. Besides the comments from my side, is this PR still blocked because of dependency issues?

[maniac103]

I still see the "help wanted" label. Besides the comments from my side, is this PR still blocked because of dependency issues?

No, it should be fine with @J-N-K's comment.

[maniac103]

AFAICT only open question that's remaining should be the channel naming discussion now.

[jlaur]

Another serialNumber round. 🙂

[jlaur]

@maniac103 - there are some checkstyle warnings left. You could take a look at target/code-analysis/report.html.

[maniac103]

there are some checkstyle warnings left. You could take a look at target/code-analysis/report.html.

Fixed, thanks for the heads-up.

[jlaur]

Thanks for your contribution!

Now, you could add your binding's logo to the openHAB website. See https://www.openhab.org/docs/developer/bindings/#add-your-binding-s-logo-to-the-openhab-website

[maniac103]

Now, you could add your binding's logo to the openHAB website.

Done: openhab/openhab-docs#2035
Thanks for the review and merging!

[JohannesPtaszyk]

Happy to see how much you invested into this after the initial poc we built together 😍
Well done! Keep the great work and thank you so much for this contribution!