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

[fronius] fix powerflowchannelpakku documentation #15381

Merged
merged 2 commits into from
Sep 10, 2023
Merged

[fronius] fix powerflowchannelpakku documentation #15381

merged 2 commits into from
Sep 10, 2023

Conversation

marcquark
Copy link
Contributor

fixes #15074

@marcquark marcquark requested a review from trokohl as a code owner August 7, 2023 20:57
@marcquark
[fronius] fix powerflowchannelpakku documentation
beda4f6 
fixes #15074

Signed-off-by: Marc Bartelt <github@mleuser.de>
@lsiepel
Copy link
Contributor

lsiepel commented Aug 7, 2023 •
edited
Loading

fixes #15074

Don't know what is good or wrong, but it is not making it more clear. Most systems show a + when charging/increasing and a - when discharging/decreasing.

The label is about the battery, so from the battery perspective if it is charging / increasing, so i would say a + is the best option. (as how it was).

@marcquark
Copy link
Contributor Author

Agree that it would be more intuitive to have a positive number (+) when charging and a negative number (-) when discharging. However the inverter's API produces the opposite. So right now the documentation is wrong.

The other option is to multiply the value by -1 to make the documentation correct, but not sure if that's a smart thing to do considering it only affects one particular property and all the others do not alter the API output.

@lsiepel
Copy link
Contributor

lsiepel commented Aug 8, 2023

Agree that it would be more intuitive to have a positive number (+) when charging and a negative number (-) when discharging. However the inverter's API produces the opposite. So right now the documentation is wrong.

The other option is to multiply the value by -1 to make the documentation correct, but not sure if that's a smart thing to do considering it only affects one particular property and all the others do not alter the API output.

Might be counter intuitive, but agree better to follow the API.

@marcquark @jimtng
Update bundles/org.openhab.binding.fronius/README.md
f28e47d 
Co-authored-by: jimtng <2554958+jimtng@users.noreply.github.com>
Signed-off-by: Marc Bartelt <marcquark@users.noreply.github.com>
lolodomo
lolodomo approved these changes Sep 10, 2023
View reviewed changes
Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

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

LGTM

@lolodomo
Copy link
Contributor

This will make the documentation in sync with channel description:
https://github.com/openhab/openhab-addons/blob/main/bundles/org.openhab.binding.fronius/src/main/resources/OH-INF/thing/thing-types.xml#L283

@lolodomo lolodomo merged commit 43ed48d into openhab:main Sep 10, 2023
@lolodomo lolodomo added this to the 4.1 milestone Sep 10, 2023
Pshatsillo pushed a commit to Pshatsillo/openhab-addons that referenced this pull request Sep 29, 2023
@marcquark @Pshatsillo
[fronius] fix powerflowchannelpakku documentation (openhab#15381)
f6f2165 
* [fronius] fix powerflowchannelpakku documentation
fixes openhab#15074

---------

Signed-off-by: Marc Bartelt <github@mleuser.de>
Signed-off-by: Marc Bartelt <marcquark@users.noreply.github.com>
pat-git023 pushed a commit to pat-git023/openhab-addons that referenced this pull request Oct 13, 2023
@marcquark @pat-git023
[fronius] fix powerflowchannelpakku documentation (openhab#15381)
1694d3e 
* [fronius] fix powerflowchannelpakku documentation
fixes openhab#15074

---------

Signed-off-by: Marc Bartelt <github@mleuser.de>
Signed-off-by: Marc Bartelt <marcquark@users.noreply.github.com>
querdenker2k pushed a commit to querdenker2k/openhab-addons that referenced this pull request Oct 21, 2023
@marcquark @querdenker2k
[fronius] fix powerflowchannelpakku documentation (openhab#15381)
6446f2e 
* [fronius] fix powerflowchannelpakku documentation
fixes openhab#15074

---------

Signed-off-by: Marc Bartelt <github@mleuser.de>
Signed-off-by: Marc Bartelt <marcquark@users.noreply.github.com>
querdenker2k pushed a commit to querdenker2k/openhab-addons that referenced this pull request Oct 29, 2023
@marcquark @querdenker2k
[fronius] fix powerflowchannelpakku documentation (openhab#15381)
998faab 
* [fronius] fix powerflowchannelpakku documentation
fixes openhab#15074

---------

Signed-off-by: Marc Bartelt <github@mleuser.de>
Signed-off-by: Marc Bartelt <marcquark@users.noreply.github.com>
Signed-off-by: querdenker2k <querdenker2k@gmx.de>
austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
@marcquark @austvik
[fronius] fix powerflowchannelpakku documentation (openhab#15381)
5cafc43 
* [fronius] fix powerflowchannelpakku documentation
fixes openhab#15074

---------

Signed-off-by: Marc Bartelt <github@mleuser.de>
Signed-off-by: Marc Bartelt <marcquark@users.noreply.github.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

@lolodomo lolodomo lolodomo approved these changes

@jimtng jimtng jimtng approved these changes

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

Assignees
No one assigned
Labels
documentation
Projects
None yet
Milestone
4.1
Development

Successfully merging this pull request may close these issues.

[fronius] powerflowchannelpakku upside down description
5 participants
@marcquark @lsiepel @lolodomo @jimtng @jlaur