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

[omnikinverter] Add channels for voltage and current #11645

Merged
merged 1 commit into from
Dec 15, 2021

Conversation

hansbogert
Copy link
Contributor

@hansbogert hansbogert commented Nov 28, 2021

Novel Addition: Add multiple channels for:

  • input voltage for string 1,2, and 3
  • input current for string 1, 2 and 3
  • output power for feed 2 and 3 as well

These data was always already in the network packet returned by the inverter, so no new retrieval code was necessary, only extra decoding of the binary packet coming from the inverter.

For the reviewer:

Please keep the following in mind:

  • Did you describe new features for the end user?

Is this other than the README.md?

  • Did you describe any noteworthy changes in usage for the end user?

Is this other than the README.md?

  • Was the documentation updated accordingly, e.g. the add-on README?

@wborn wborn added enhancement An enhancement or new feature for an existing add-on work in progress A PR that is not yet ready to be merged labels Nov 28, 2021
@hansbogert hansbogert changed the title [WIP][omnikinverter] feature: Add channels for voltage and current [omnikinverter] feature: Add channels for voltage and current Nov 28, 2021
@hansbogert
Copy link
Contributor Author

@wborn not sure how to remove the "work in progress" tag, if that's to-be changed be committer in the first place. Either way, I think a review can be done in the current state. Please also see my own review comments, which are basically questions to the reviewer.

@wborn wborn removed the work in progress A PR that is not yet ready to be merged label Nov 29, 2021
@wborn
Copy link
Member

wborn commented Nov 29, 2021

Thanks for the update! I've removed the label. 🙂

@hansbogert hansbogert force-pushed the main branch 2 times, most recently from 7f102fc to 23d904d Compare December 12, 2021 21:05
@hansbogert
Copy link
Contributor Author

Not sure if I should explicitly ask for (re)review, so just a gentle ping @lolodomo

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 lolodomo merged commit 4291729 into openhab:main Dec 15, 2021
@lolodomo lolodomo added this to the 3.2 milestone Dec 15, 2021
@wborn wborn changed the title [omnikinverter] feature: Add channels for voltage and current [omnikinverter] Add channels for voltage and current Dec 18, 2021
NickWaterton pushed a commit to NickWaterton/openhab-addons that referenced this pull request Dec 30, 2021
mischmidt83 pushed a commit to mischmidt83/openhab-addons that referenced this pull request Jan 9, 2022
nemerdaud pushed a commit to nemerdaud/openhab-addons that referenced this pull request Jan 28, 2022
marcfischerboschio pushed a commit to bosch-io/openhab-addons that referenced this pull request May 5, 2022
andan67 pushed a commit to andan67/openhab-addons that referenced this pull request Nov 6, 2022
andrasU pushed a commit to andrasU/openhab-addons that referenced this pull request Nov 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants