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] Fix jinja usage in availability templates #17400

Merged
merged 2 commits into from
Sep 27, 2024

Conversation

ccutrer
Copy link
Contributor

@ccutrer ccutrer commented Sep 10, 2024

Use the local Jinjava, instead of implicitly depending on the Jinja transformation service.

This was missed in the recent conversion to using Jinjava for transformations in "normal" channels.

Use the local Jinjava, instead of implicitly depending on the Jinja
transformation service.

Signed-off-by: Cody Cutrer <[email protected]>
@ccutrer ccutrer requested review from jimtng, J-N-K and lsiepel September 10, 2024 16:12
@ccutrer ccutrer requested a review from antroids as a code owner September 10, 2024 16:12
@ccutrer ccutrer added the bug An unexpected problem or unintended behavior of an add-on label Sep 10, 2024
@lsiepel
Copy link
Contributor

lsiepel commented Sep 10, 2024

Test fails.

@ccutrer
Copy link
Contributor Author

ccutrer commented Sep 10, 2024

Yes, but I don't understand why. The Homie itests are failing that it's unable to load the ChannelTransformation class, but that class is in the org.openhab.core.thing bundle. How on earth would that bundle not be available??

@lsiepel
Copy link
Contributor

lsiepel commented Sep 11, 2024

Yes, but I don't understand why. The Homie itests are failing that it's unable to load the ChannelTransformation class, but that class is in the org.openhab.core.thing bundle. How on earth would that bundle not be available??

Spend over an hour looking at this, but i also don;t understand why this is failing. Maybe @wborn can add anything as he knows way more about the itest's (no pressure ;-) )

@ccutrer
Copy link
Contributor Author

ccutrer commented Sep 12, 2024

I was able to get it to pass locally by importing ChannelTransformation in HomieThingHandler.java, (and adding a dummy method so Spotless doesn't remove the import):

    private void doNothing(ChannelTransformation transform) {
        System.out.println("Transform: " + transform.toString());
    }

Obviously that's not a proper solution, but it points to something about the Homie bundle not listing the ChannelTransformation class as a necessary import or something. I'm waaaay out of my Java depth with this, though.

@ccutrer
Copy link
Contributor Author

ccutrer commented Sep 17, 2024

@wborn: have you had a chance to look at this?

@wborn
Copy link
Member

wborn commented Sep 19, 2024

I think it's some Mockito bug. We could try upgrading Mockito and see if that helps.

@ccutrer
Copy link
Contributor Author

ccutrer commented Sep 20, 2024

I tried, but ran into the same thing as you! mockito/mockito#2708

@ccutrer
Copy link
Contributor Author

ccutrer commented Sep 27, 2024

Given that it seems like a Mockito upgrade is some ways away, I've added my dummy method workaround to get this PR through. We can revisit removing it if/when Mockito gets upgraded in core.

Copy link
Contributor

@lsiepel lsiepel left a comment

Choose a reason for hiding this comment

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

Thanks, the dummy method is a small price to move forward. It is well documented so LGTM

@lsiepel lsiepel merged commit f4805ed into openhab:main Sep 27, 2024
4 of 5 checks passed
@lsiepel lsiepel added this to the 4.3 milestone Sep 27, 2024
@ccutrer ccutrer deleted the mqtt-homeassistant-jinjava-availability branch September 27, 2024 14:53
joni1993 pushed a commit to joni1993/openhab-addons that referenced this pull request Oct 15, 2024
…ab#17400)

* [mqtt.homeassistant] Fix jinja usage in availability templates

Use the local Jinjava, instead of implicitly depending on the Jinja
transformation service.

Signed-off-by: Cody Cutrer <[email protected]>
matchews pushed a commit to matchews/openhab-addons that referenced this pull request Oct 18, 2024
…ab#17400)

* [mqtt.homeassistant] Fix jinja usage in availability templates

Use the local Jinjava, instead of implicitly depending on the Jinja
transformation service.

Signed-off-by: Cody Cutrer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem or unintended behavior of an add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants