[mqtt] HA: add transformation to incoming topics, where needed #4990

jochen314 · 2019-02-24T21:15:10Z

This change adds a transformation from a value_template to an incoming message, where needed.

As I do not have a working PaperUI right now, I could not fully test this step including the front-end...

davidgraeff · 2019-02-24T21:25:40Z

...rg/openhab/binding/mqtt/generic/internal/convention/homeassistant/ComponentBinarySensor.java

-        channels.put(sensorChannelID,
-                new CChannel(this, sensorChannelID, new OnOffValue(config.payload_on, config.payload_off),
-                        config.state_topic, null, config.name, config.unit_of_measurement, channelStateUpdateListener));
+        CChannel sensorChannel = new CChannel(this, sensorChannelID,


Note to myself: Use a channel builder pattern

I thought about this also.
One of my next request would add specialiezed constructors for read only/write only channels.
But I will thing this through again.

davidgraeff

LGTM

davidgraeff · 2019-02-24T21:33:19Z

If you are ready, can you remove the [WIP] for others to perform a review? Oh, and Travis fails atm

jochen314 · 2019-02-24T21:52:33Z

I have not actually run the code in openhab and test it in the gui. That is why I put the wip there in the first place.
It seems, that the ui migration is not finished yet, right?
Is it ok to remove WIP without the testings?

davidgraeff · 2019-02-24T23:19:54Z

Could you, if possible, not force push, please. As a reviewer you never know what has changed from the last review otherwise. We can at the end merge join all commits.

A UI test can be fully replaced by an equivalent integration test. I do even prefer that.

@wborn had success building the recent state, including UIs, if I've read him correctly in the forum

wborn · 2019-02-24T23:49:04Z

In the latest snapshots builds (1534,1535) the UIs are working again.

They also worked while launching the runtime in my Eclipse today. You might need to build/import the openhab-webui projects for that locally.

davidgraeff · 2019-03-14T11:41:37Z

@jochen314 Can you notify me when we start to tackle this pr? Thanks.

Signed-off-by: Jochen Klein <git@jochen.susca.de>

jochen314 · 2019-03-16T17:35:08Z

I think, this is ready now.

davidgraeff · 2019-03-16T17:45:45Z

...c/main/java/org/openhab/binding/mqtt/generic/internal/convention/homeassistant/CFactory.java

-     *            "{name:'Name',state_topic:'homeassistant/switch/0/object/state',command_topic:'homeassistant/switch/0/object/set'".
+     * @param thingUID       The Thing UID that this component will belong to.
+     * @param haID           The location of this component. The HomeAssistant ID contains the object-id, node-id and
+     *                           component-id.


Not a problem for me, but for others probably. Go to eclipse window->preferences->Java->code style->formatter and make sure that "ESH" is the active profile. Click apply, even if it is. Then go back to this file and let it be reformatted again.

davidgraeff · 2019-03-16T17:53:18Z

Some unrelated problems in travis:

[ERROR] Cannot resolve project dependencies:
[ERROR]   Software being installed: org.openhab.binding.plclogo 2.5.0.qualifier
[ERROR]   Missing requirement: org.eclipse.xtext.util 2.17.0.v20190304-0545 requires 'osgi.bundle; com.google.guava [21.0.0,22.0.0)' but it could not be found
[ERROR]   Cannot satisfy dependency: org.eclipse.xtext 2.17.0.v20190304-0545 depends on: osgi.bundle; org.eclipse.xtext.util 0.0.0
[ERROR]   Cannot satisfy dependency: org.openhab.binding.plclogo 2.5.0.qualifier depends on: java.package; org.eclipse.smarthome.model.script.actions 0.0.0
[ERROR]   Cannot satisfy dependency: org.openhab.core.model.item 2.5.0.201903150831 depends on: osgi.bundle; org.eclipse.xtext 0.0.0
[ERROR]   Cannot satisfy dependency: org.openhab.core.model.script 2.5.0.201903150823 depends on: osgi.bundle; org.openhab.core.model.item 0.0.0
[ERROR]

There was a xtext update in openhab-distro afaik, not sure. I'll retrigger in a day.

Signed-off-by: Jochen Klein <git@jochen.susca.de>

davidgraeff

@openhab/2-x-add-ons-maintainers Can someone have a second look? Before migrating mqtt, all PRs need to be in. Thanks.

martinvw

One minor comment, thanks!

martinvw · 2019-03-19T13:30:45Z

...va/org/openhab/binding/mqtt/generic/internal/convention/homeassistant/AbstractComponent.java

@@ -41,6 +42,7 @@
 @NonNullByDefault
 public abstract class AbstractComponent<C extends BaseChannelConfiguration> {
    // Component location fields
+    private ComponentConfiguration componentConfiguration;


Any reason why its not final?

Dis you also see this one?

Can you merge this PR nevertheless please? There are 1-5 more PRs to come and this code will be touched again anyhow.

The initial 3000 LOC PR was presented a month ago now and allows us to be almost 100% MQTT HA compliant and I asked Jochen to split it up into smaller pieces, easier to review for me.
And he did and his PRs are of really good quality.

There will always be the one small imperfection somewhere, but I will make sure that it is always functional correct and resource efficient.

Please let me do my job as mqtt maintainer and decide when to merge a PR, as I know the greater plan for this module. (It's not actually a critic towards you Martin, but the general process that I observe here, where contributors loose interest, because the patch flow is slowed down for nitpick reasons)

@martinvw Yes, I saw it, but I could not fix it while at work.
Not special reason, just a late addition and I forgot to make it final.
I have no problem to change it ;-)

Given you responded on the other one I just wanted to make sure that we could wrap up quick I did not realize more would be commng soon

...rg/openhab/binding/mqtt/generic/internal/convention/homeassistant/ComponentBinarySensor.java

Signed-off-by: Jochen Klein <git@jochen.susca.de>

…ab#4990) Signed-off-by: Jochen Klein <git@jochen.susca.de> Signed-off-by: Pshatsillo <pshatsillo@gmail.com>

…ab#4990) Signed-off-by: Jochen Klein <git@jochen.susca.de> Signed-off-by: Maximilian Hess <mail@ne0h.de>

jochen314 requested a review from davidgraeff as a code owner February 24, 2019 21:15

davidgraeff reviewed Feb 24, 2019

View reviewed changes

davidgraeff approved these changes Feb 24, 2019

View reviewed changes

jochen314 force-pushed the addValueTransformation branch from 20b77be to 0e78aae Compare February 24, 2019 21:49

wborn added the mqtt label Feb 24, 2019

wborn added the work in progress A PR that is not yet ready to be merged label Feb 25, 2019

jochen314 mentioned this pull request Mar 3, 2019

[mqtt] Add support for abbreviations and '~' expansion in home assistant auto discovery #5035

Closed

jochen314 force-pushed the addValueTransformation branch from 23bb3fc to e645ba4 Compare March 3, 2019 17:19

jochen314 mentioned this pull request Mar 7, 2019

[mqtt] Add support for abbreviations and '~' expansion in home assistant auto discovery #5065

Merged

davidgraeff approved these changes Mar 10, 2019

View reviewed changes

davidgraeff added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Mar 10, 2019

rebase

fea4a42

Signed-off-by: Jochen Klein <git@jochen.susca.de>

jochen314 force-pushed the addValueTransformation branch from e932486 to fea4a42 Compare March 15, 2019 20:05

jochen314 changed the title ~~[mqtt][WIP] HA: add transformation to incoming topics, where needed~~ [mqtt] HA: add transformation to incoming topics, where needed Mar 16, 2019

davidgraeff reviewed Mar 16, 2019

View reviewed changes

jochen314 added 2 commits March 16, 2019 20:29

formatting

4144400

Signed-off-by: Jochen Klein <git@jochen.susca.de>

add missing copyright headers

b025f10

Signed-off-by: Jochen Klein <git@jochen.susca.de>

davidgraeff approved these changes Mar 18, 2019

View reviewed changes

davidgraeff mentioned this pull request Mar 18, 2019

[mqtt][WIP] Make Homeassistant discovery work for Tasmota #4927

Closed

cweitkamp removed the work in progress A PR that is not yet ready to be merged label Mar 18, 2019

martinvw reviewed Mar 19, 2019

View reviewed changes

review finding

4e60fa8

Signed-off-by: Jochen Klein <git@jochen.susca.de>

jochen314 mentioned this pull request Mar 19, 2019

[mqtt] Have a working Group Id #5156

Merged

martinvw approved these changes Mar 19, 2019

View reviewed changes

martinvw merged commit 0ea0bd1 into openhab:master Mar 19, 2019

wborn added this to the 2.5 milestone Apr 14, 2019

jochen314 deleted the addValueTransformation branch August 22, 2019 14:44

ne0h pushed a commit to ne0h/openhab-addons that referenced this pull request Sep 15, 2019

[mqtt] HA: add transformation to incoming topics, where needed (openh…

ee4d739

…ab#4990) Signed-off-by: Jochen Klein <git@jochen.susca.de> Signed-off-by: Maximilian Hess <mail@ne0h.de>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[mqtt] HA: add transformation to incoming topics, where needed #4990

[mqtt] HA: add transformation to incoming topics, where needed #4990

jochen314 commented Feb 24, 2019

davidgraeff Feb 24, 2019

jochen314 Feb 24, 2019

davidgraeff left a comment

davidgraeff commented Feb 24, 2019

jochen314 commented Feb 24, 2019

davidgraeff commented Feb 24, 2019

wborn commented Feb 24, 2019

davidgraeff commented Mar 14, 2019

jochen314 commented Mar 16, 2019

davidgraeff Mar 16, 2019

davidgraeff commented Mar 16, 2019 •

edited

Loading

davidgraeff left a comment

martinvw left a comment

martinvw Mar 19, 2019

martinvw Mar 19, 2019

davidgraeff Mar 19, 2019 •

edited

Loading

jochen314 Mar 19, 2019

martinvw Mar 19, 2019

[mqtt] HA: add transformation to incoming topics, where needed #4990

[mqtt] HA: add transformation to incoming topics, where needed #4990

Conversation

jochen314 commented Feb 24, 2019

davidgraeff Feb 24, 2019

Choose a reason for hiding this comment

jochen314 Feb 24, 2019

Choose a reason for hiding this comment

davidgraeff left a comment

Choose a reason for hiding this comment

davidgraeff commented Feb 24, 2019

jochen314 commented Feb 24, 2019

davidgraeff commented Feb 24, 2019

wborn commented Feb 24, 2019

davidgraeff commented Mar 14, 2019

jochen314 commented Mar 16, 2019

davidgraeff Mar 16, 2019

Choose a reason for hiding this comment

davidgraeff commented Mar 16, 2019 • edited Loading

davidgraeff left a comment

Choose a reason for hiding this comment

martinvw left a comment

Choose a reason for hiding this comment

martinvw Mar 19, 2019

Choose a reason for hiding this comment

martinvw Mar 19, 2019

Choose a reason for hiding this comment

davidgraeff Mar 19, 2019 • edited Loading

Choose a reason for hiding this comment

jochen314 Mar 19, 2019

Choose a reason for hiding this comment

martinvw Mar 19, 2019

Choose a reason for hiding this comment

davidgraeff commented Mar 16, 2019 •

edited

Loading

davidgraeff Mar 19, 2019 •

edited

Loading