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

[thing] Added default channel types for electrical energy #3079

Merged
merged 2 commits into from
Sep 11, 2022

Conversation

cweitkamp
Copy link
Contributor

  • Added default channel types for electrical energy (electric power, electric current, electric potential and electrical energy)

Signed-off-by: Christoph Weitkamp github@christophweitkamp.de

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
@cweitkamp cweitkamp added the enhancement An enhancement or new feature of the Core label Sep 8, 2022
@cweitkamp cweitkamp requested a review from a team as a code owner September 8, 2022 18:38
Comment on lines 383 to 388
public static final ChannelType SYSTEM_ELECTRIC_POTENTIAL = ChannelTypeBuilder
.state(SYSTEM_CHANNEL_TYPE_UID_ELECTRIC_POTENTIAL, "Electric Potential", "Number:ElectricPotential")
.withDescription("Current electric potential").withCategory("Energy")
.withStateDescriptionFragment(
StateDescriptionFragmentBuilder.create().withReadOnly(true).withPattern("%.1f %unit%").build())
.withTags(List.of("Measurement", "Voltage")).build();
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure that this is a good naming? Usually it'll probably be a potential difference (aka voltage), not na absolute potential.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I am not. I tried to create a relation to the dimension to be used with the ItemType and a cluster for electrical Channels. In general we can name all of them just Current, Power, Voltage and Energy - without the prefix.

Copy link
Member

Choose a reason for hiding this comment

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

IMO that would be better from the user perspective.

Copy link
Contributor Author

@cweitkamp cweitkamp Sep 10, 2022

Choose a reason for hiding this comment

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

Me too.

But there is one problem. The channel type power already is used for turning devices on and off:

public static final ChannelTypeUID SYSTEM_CHANNEL_TYPE_UID_POWER = new ChannelTypeUID(BINDING_ID, "power");

Copy link
Member

Choose a reason for hiding this comment

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

Maybe keep the "Electric" prefix then. There will probably be no clash for "voltage" but "current" could also be related to water or gas.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay. Then I will rename just electric potential to electric voltage and keep the rest.

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
@J-N-K J-N-K merged commit cda7b09 into openhab:main Sep 11, 2022
@J-N-K J-N-K added this to the 3.4 milestone Sep 11, 2022
@cweitkamp cweitkamp deleted the feature-energy-channel-types branch September 11, 2022 12:55
splatch pushed a commit to ConnectorIO/copybara-hab-core that referenced this pull request Jul 12, 2023
* Added default channel types for electrical energy

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
GitOrigin-RevId: cda7b09
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 of the Core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants