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.

Sign up for GitHub

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

Jump to bottom

[tesla] Add channels for software update #15816

Merged
merged 13 commits into from
Nov 1, 2023
Merged

[tesla] Add channels for software update #15816

merged 13 commits into from
Nov 1, 2023

Conversation

hakan42
Copy link
Contributor

@hakan42 hakan42 commented Oct 28, 2023 •
edited
Loading

With this enhancement, the software update information of the cars can be injected into OH.

The PR has been running on my local instance for weeks now, waiting for the next software update. Finally, with the upgrade from 2023.32.6 to 2023.32.9, I could confirm that the "update available" OnOff-Channel works as intended, too.

The "carversion" property of the things had to be disabled to make the enhancement work with logic the TeslaChannelSelectorProxy. If this is used by someone, I'll try to re-add the property in an upcoming pull request.

@hakan42
[tesla] Add value holders for Software Update state
35d5b78 
Signed-off-by: Hakan Tandogan <hakan@tandogan.com>
@hakan42
[tesla] Add channels for software update, including a switch for avai…
66442d0 
…lability

Signed-off-by: Hakan Tandogan <hakan@tandogan.com>
@hakan42
[tesla] Update instructions for software update channels
5143107 
Signed-off-by: Hakan Tandogan <hakan@tandogan.com>
@hakan42 hakan42 requested a review from kgoderis as a code owner October 28, 2023 10:46
@hakan42
[tesla] Update documentation for software update channels
bd6ad93 
Signed-off-by: Hakan Tandogan <hakan@tandogan.com>
@hakan42
Copy link
Contributor Author

hakan42 commented Oct 28, 2023

@jlaur , may I draw your attention to this pull request? Would be extremely nice if it could be incorporated into the next minor release :)

@lsiepel lsiepel added the enhancement An enhancement or new feature for an existing add-on label Oct 28, 2023
lsiepel
lsiepel requested changes Oct 28, 2023
View reviewed changes
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.

Looks good, do have some suggestions and questions.

bundles/org.openhab.binding.tesla/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.tesla/README.md Outdated Show resolved Hide resolved
...inding.tesla/src/main/java/org/openhab/binding/tesla/internal/TeslaChannelSelectorProxy.java Outdated Show resolved Hide resolved
bundles/org.openhab.binding.tesla/src/main/resources/OH-INF/thing/channels.xml Show resolved Hide resolved
@lsiepel lsiepel requested a review from a team October 28, 2023 16:00
@hakan42
[tesla] Review comment - keep 'version' as a property, not channel
05ad0e2 
Signed-off-by: Hakan Tandogan <hakan@tandogan.com>
@hakan42 hakan42 requested a review from lsiepel October 28, 2023 19:07
lsiepel
lsiepel requested changes Oct 29, 2023
View reviewed changes
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.

Minor textual findings. otherwise looks good.

bundles/org.openhab.binding.tesla/src/main/resources/OH-INF/thing/channels.xml Outdated Show resolved Hide resolved
bundles/org.openhab.binding.tesla/src/main/resources/OH-INF/thing/channels.xml Outdated Show resolved Hide resolved
bundles/org.openhab.binding.tesla/src/main/resources/OH-INF/thing/channels.xml Outdated Show resolved Hide resolved
bundles/org.openhab.binding.tesla/src/main/resources/OH-INF/i18n/tesla.properties Outdated

# channel types

channel-type.tesla.softwareversion.label = Software version
Copy link
Contributor

Choose a reason for hiding this comment

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

Think the i18n files need to be regenerated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done and pushed.

Copy link
Contributor

Choose a reason for hiding this comment

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

What am i missing, i would not expect this as software version is no longer a channel, but a property as it was before.

hakan42 and others added 4 commits October 29, 2023 11:57
@hakan42 @lsiepel
Update bundles/org.openhab.binding.tesla/src/main/resources/OH-INF/th…
a8d58fc 
…ing/channels.xml

Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Hakan Tandogan <hakan@tandogan.com>
@hakan42 @lsiepel
Update bundles/org.openhab.binding.tesla/src/main/resources/OH-INF/th…
2d5cbdc 
…ing/channels.xml

Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Hakan Tandogan <hakan@tandogan.com>
@hakan42 @lsiepel
Update bundles/org.openhab.binding.tesla/src/main/resources/OH-INF/th…
3225f85 
…ing/channels.xml

Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Hakan Tandogan <hakan@tandogan.com>
@hakan42
[tesla] regenerated i18n files after review comments
2a0fb83 
Signed-off-by: Hakan Tandogan <hakan@tandogan.com>
@hakan42
Copy link
Contributor Author

hakan42 commented Oct 29, 2023

@jlaur, thank you for your review, I think I addressed all your comments. Could you give it another twirl real quick if time permits?

lsiepel
lsiepel approved these changes Oct 29, 2023
View reviewed changes
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.

LGTM, one comment is open regarding channel-type softwareversion in i18n file.

btw, i consider mixing me up with @jlaur as a big compliment ;-)

bundles/org.openhab.binding.tesla/src/main/resources/OH-INF/i18n/tesla.properties Outdated

# channel types

channel-type.tesla.softwareversion.label = Software version
Copy link
Contributor

Choose a reason for hiding this comment

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

What am i missing, i would not expect this as software version is no longer a channel, but a property as it was before.

@hakan42
[tesla] Removed superflous softwareversion channel description, as it…
3b78b22 
… is a property again

Signed-off-by: Hakan Tandogan <hakan@tandogan.com>
@hakan42
Copy link
Contributor Author

hakan42 commented Oct 29, 2023

LGTM, one comment is open regarding channel-type softwareversion in i18n file.

Oops, removed it from the i18n file, like it is in the main branch as well.

btw, i consider mixing me up with @jlaur as a big compliment ;-)

See how I should have more coffee before writing comments :)

lsiepel
lsiepel approved these changes Oct 29, 2023
View reviewed changes
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.

LGTM

kaikreuzer
kaikreuzer approved these changes Nov 1, 2023
View reviewed changes
Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

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

Thanks!

bundles/org.openhab.binding.tesla/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.tesla/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.tesla/README.md Outdated Show resolved Hide resolved
@kaikreuzer
Update bundles/org.openhab.binding.tesla/README.md
0132d1d 
Signed-off-by: Kai Kreuzer <kai@openhab.org>
@kaikreuzer
Update bundles/org.openhab.binding.tesla/README.md
3e1ad6c 
Signed-off-by: Kai Kreuzer <kai@openhab.org>
@kaikreuzer
Update bundles/org.openhab.binding.tesla/README.md
a050024 
Signed-off-by: Kai Kreuzer <kai@openhab.org>
@kaikreuzer kaikreuzer added this to the 4.1 milestone Nov 1, 2023
@kaikreuzer kaikreuzer merged commit 59c3135 into openhab:main Nov 1, 2023
2 checks passed
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Nov 26, 2023
@hakan42 @andrewfg
[tesla] Add channels for software update (openhab#15816)
eb1f162 
* [tesla] Add value holders for Software Update state

Signed-off-by: Hakan Tandogan <hakan@tandogan.com>
austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
@hakan42 @austvik
[tesla] Add channels for software update (openhab#15816)
f7acc73 
* [tesla] Add value holders for Software Update state

Signed-off-by: Hakan Tandogan <hakan@tandogan.com>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lsiepel lsiepel lsiepel approved these changes

@kaikreuzer kaikreuzer kaikreuzer approved these changes

@kgoderis kgoderis Awaiting requested review from kgoderis kgoderis is a code owner

Assignees
No one assigned
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Milestone
4.1
Development

Successfully merging this pull request may close these issues.
3 participants
@hakan42 @kaikreuzer @lsiepel