openhab / openhab-addons

Add-ons for openHAB
https://www.openhab.org/
Eclipse Public License 2.0
1.86k stars 3.56k forks source link

[mqtt.homeassistant] JINJA transformation should be installed when MQTT binding is installed #6875

Open bodiroga opened 4 years ago

bodiroga commented 4 years ago

Hi guys!

As the HomeAssistant integration is using the JINJA service to extract the right information from the MQTT payload (https://github.com/openhab/openhab-addons/blob/2.5.x/bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/CChannel.java#L60), I think that it should be included as a dependency to the binding. It seems to be confusing for some users, for example: https://github.com/openhab/openhab-addons/issues/6271.

Is this something that can be added to the HomeAssistant bundle POM file? (https://github.com/openhab/openhab-addons/blob/2.5.x/bundles/org.openhab.binding.mqtt.homeassistant/pom.xml)

Something like:

<dependency>
    <groupId>org.openhab.addons.bundles</groupId>
    <artifactId>org.openhab.transform.jinja</artifactId>
    <version>${project.version}</version>
    <scope>provided</scope>
</dependency>

would be enough? What do you think @J-N-K?

Best regards,

Aitor

J-N-K commented 4 years ago

This needs to be added to the feature, IIRC there is an aggregated feature for mqtt, the jinja transformation needs to be added there.

bodiroga commented 4 years ago

Here: https://github.com/openhab/openhab-addons/blob/2.5.x/features/openhab-addons/src/main/resources/footer.xml? I can make a PR adding the line <feature>openhab-transform-jinja</feature>, although I don't know how can I test it :disappointed:

J-N-K commented 4 years ago

Sounds good. It's not easy to test, though. I think you would need to exchange the feature-file in an openHAB instance and then install a new one.

bodiroga commented 4 years ago

I have found that the Home Assistant bundle has its own feature.xml file. It seems to be a better idea to edit this file instead of the generic one, isn't it?

I think you would need to exchange the feature-file in an openHAB instance and then install a new one.

Wow, that is beyond my current skills :cry:

bodiroga commented 3 years ago

Hi @Hilbrand!

I don't know if you can help us with this, but it would be amazing if we could solve it for the next openHAB release, before the change to OH3.

Should I add the line <feature>openhab-transform-jinja</feature> to https://github.com/openhab/openhab-addons/blob/2.5.x/features/openhab-addons/src/main/resources/footer.xml or https://github.com/openhab/openhab-addons/blob/2.5.x/bundles/org.openhab.binding.mqtt.homeassistant/src/main/feature/feature.xml? Would that be enough?

jimtng commented 2 years ago

@bodigra I currently have no comment on the whole matter, just wondering whether that feature name should be ninja or jinja?

bodiroga commented 2 years ago

Ups... you are right, @jimtng, it should be jinja. I have edited my two mistakes in order to avoid any future misunderstandings 👍🏻

Are you willing to take a look at it? I have no free time now, sorry 😞 Thanks for the work you are doing!

jimtng commented 2 years ago

It seems easy enough to add this but should it? Homeassistant is bundled together with the main mqtt. I use the main mqtt but not homeassistant. Therefore I don't need nor have installed the jinja transformation.

If jinja was bundled, it'd be installed on my system which is not desired.

It should be sufficient to note that jinja should be installed if one wants to use the homeassistant feature.

Alternatively, if it is feasible, split the homeassistant as a separate binding and bundle it with that. This might be the most ideal solution. Wdyt?

hmerk commented 2 years ago

Closed as there is no real need for and there will be no more changes to oh 2.5.x

ssalonen commented 1 year ago

This issue persists.

Impact: homeassistant mqtt discovery works (things are found, channels are there for the things) but actual parsing of payloads fail.

There will be regular error messages from mqtt binding [1] on the missing JINJA transformation service. Channels states and items are not updated.

Community seems to workaround this issue, by installing jinja manually. For example, see openhab 3 integration guidance for openmqttgateway https://docs.openmqttgateway.com/integrate/openhab3.html#install-the-mqtt-binding

Comment from @jimtng is still valid though, mqtt generic binding does need jinja.

EDIT: it is worth noting that Jinja and jsonpath are mentioned as dependencies in the homeassistant mqtt docs

EDIT2: for those who are wondering (like me), jinja is inherent part of homeassistant discovery spec. We probably should not consider replacing jinja transformation with something else.

[1] Example

[WARN] Transformation service JINJA for pattern {{ value_json.tx}} not found!
ssalonen commented 1 year ago

If jinja was bundled, it'd be installed on my system which is not desired.

It should be sufficient to note that jinja should be installed if one wants to use the homeassistant feature.

Alternatively, if it is feasible, split the homeassistant as a separate binding and bundle it with that. This might be the most ideal solution. Wdyt?

I think this comment summarizes the issue well.

Already now users stuff installed they might not need nor want, like the various mqtt discovery implementations, or with Bluetooth support for BLE sensors that the user do not have. Very similar situation is with modbus, that I am maintainer for. In my production setup I am very certain that I do not need any of the device specific adaptations of modbus.

They are all bundled in one mqtt, Bluetooth or modbus bundle, for user convenience.

Then again, we have different DIY projects advertising "mqtt support" (or modbus, or Bluetooth for that matter) and for novice user I think it is a great win not to go deeper into rabbit hole, understanding "mqtt conventions" and whether this follows this or that convention. Just install mqtt and you are good to go.

(case example) In fact, I am still unsure what is exactly openmqttgateway "oh discovery" they advertise, it looks very much home assistant convention to me.

Now, the obvious downside bundling these is possible disk space and memory use. As we talk about transformations, I think the added overhead is minimal, though I have not checked this.

In the case of home assistant discovery, I would argue that much of runtime overhead is already there (for all mqtt users), since, to my understanding, the discovery does not need Jinja or jsonpath transformations to work, only the parsing of values of configured things.

Where do we set the line here? What are the thresholds for deciding to e.g. Split bundles into several? If we really want, it might be even possible to have "all in one mqtt" AND separate bundles like "homie", "home assistant" etc.