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

[mybmw] Improve data refresh handling #16418

Merged
merged 12 commits into from Feb 25, 2024
Merged

[mybmw] Improve data refresh handling #16418

merged 12 commits into from Feb 25, 2024

Conversation

martingrassl
Copy link
Contributor

@martingrassl martingrassl commented Feb 16, 2024 •
edited by lsiepel

This PR will add some functionalities:

  • disable automated refresh of the vehicle data by setting the refresh interval to 0
  • enable manual refresh by forcing update of parts of the vehicle data (state, charging statistics, image)

Resolves: #16351

@martingrassl
[mybmw] add functionality for updating
f092f0e 
disable updating by setting the refresh-interval to 0
enable force update by adding some switches

Signed-off-by: Martin Grassl <martin.grassl@digital-filestore.de>
@martingrassl
[mybmw] fixed issue with reloading,
dbdb710 
added translations

Signed-off-by: Martin Grassl <martin.grassl@digital-filestore.de>
@martingrassl
[mybmw] update documentation with new channels
4465302 
for forcing update

Signed-off-by: Martin Grassl <martin.grassl@digital-filestore.de>
lsiepel
lsiepel requested changes Feb 17, 2024
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.

Thanks for the improvements. Left some comments.

bundles/org.openhab.binding.mybmw/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.mybmw/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.mybmw/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.mybmw/README.md Outdated Show resolved Hide resolved
...g.openhab.binding.mybmw/src/main/java/org/openhab/binding/mybmw/internal/MyBMWConstants.java Show resolved Hide resolved
bundles/org.openhab.binding.mybmw/src/main/resources/OH-INF/update/thing-update.xml Show resolved Hide resolved
@martingrassl
[mybmw] fix markdown table alignment
16b8e2f 
Signed-off-by: Martin Grassl <martin.grassl@digital-filestore.de>
@martingrassl
[mybmw] remove channel labels in documentation
d563cd3 
Signed-off-by: Martin Grassl <martin.grassl@digital-filestore.de>
@martingrassl
[mybmw] extend documentation about new channels
0179580 
Signed-off-by: Martin Grassl <martin.grassl@digital-filestore.de>
@martingrassl
[mybmw] re-add update instructions for version 1
273fdff 
Signed-off-by: Martin Grassl <martin.grassl@digital-filestore.de>
lsiepel
lsiepel requested changes Feb 17, 2024
View reviewed changes
Copy link
Contributor

@lsiepel lsiepel left a comment •
edited

Choose a reason for hiding this comment

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

Changes look good. Last minor comment.

bundles/org.openhab.binding.mybmw/README.md Outdated Show resolved Hide resolved
@lsiepel lsiepel added the enhancement An enhancement or new feature for an existing add-on label Feb 17, 2024
@lsiepel lsiepel changed the title [mybmw] 16351 improve polling functionality [mybmw] Improve data refresh handling Feb 17, 2024
@martingrassl @lsiepel
Formatting of README.md
d4f567e 
Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Martin Grassl <martin.grassl@digital-filestore.de>
lsiepel
lsiepel approved these changes Feb 17, 2024
View reviewed changes
Copy link
Contributor

@lsiepel lsiepel left a comment •
edited

Choose a reason for hiding this comment

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

LGTM, @mherwege or @ntruchsess any comments before merge?

mherwege
mherwege reviewed Feb 19, 2024
View reviewed changes
bundles/org.openhab.binding.mybmw/README.md Outdated
| Fuel Range Radius | range-radius-fuel | Number:Length | X | X | X | |
| Electric Range Radius | range-radius-electric | Number:Length | | X | X | X |
| Hybrid Range Radius | range-radius-hybrid | Number:Length | | X | X | |
| Channel ID | Type | conv | phev | bev_rex | bev |
Copy link
Contributor

@mherwege mherwege Feb 19, 2024 •
edited

Choose a reason for hiding this comment

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

I am OK removing the label, but I would then make sure there is a description everywhere. E.g. soc may not be easily understood. I think you can move the label in this table to a description column, making it consistent everywhere.
The same is true everywhere you removed label where there is no description column.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right I'll add some documentation

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Much better, but maybe for consistency, capitalize the first letter in each description (or remove capital everywhere)?

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

martingrassl and others added 4 commits February 19, 2024 22:58
@martingrassl @mherwege
Update bundles/org.openhab.binding.mybmw/README.md
42e3090 
Co-authored-by: Mark Herwege <mherwege@users.noreply.github.com>
Signed-off-by: Martin Grassl <martin.grassl@digital-filestore.de>
@martingrassl
[mybmw] update documentation
6e34482 
Signed-off-by: Martin Grassl <martin.grassl@digital-filestore.de>
@martingrassl
[mybmw] set autoUpdatePolicy to veto
d204c7d 
Signed-off-by: Martin Grassl <martin.grassl@digital-filestore.de>
@martingrassl
[mybmw] capitalize descriptions
c7c6a1f 
Signed-off-by: Martin Grassl <martin.grassl@digital-filestore.de>
lsiepel
lsiepel approved these changes Feb 24, 2024
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

mherwege
mherwege approved these changes Feb 25, 2024
View reviewed changes
Copy link
Contributor

@mherwege mherwege left a comment

Choose a reason for hiding this comment

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

LGTM

@martingrassl
Copy link
Contributor Author

LGTM

Thanks!

@lsiepel lsiepel merged commit d56c9e5 into openhab:main Feb 25, 2024
3 checks passed
@lsiepel lsiepel added this to the 4.2 milestone Feb 25, 2024
austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
@martingrassl @austvik
[mybmw] Improve data refresh handling (openhab#16418)
9a48113 
* [mybmw] add functionality for updating
disable updating by setting the refresh-interval to 0
enable force update by adding some switches

Signed-off-by: Martin Grassl <martin.grassl@digital-filestore.de>
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

@mherwege mherwege mherwege approved these changes

@ntruchsess ntruchsess Awaiting requested review from ntruchsess ntruchsess 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.2
Development

Successfully merging this pull request may close these issues.

[mybmw] make update interval more flexible and introduce manual updates
3 participants
@martingrassl @lsiepel @mherwege