Skip to content

Add Mega Joule as valid unit of energy#86055

Merged
epenet merged 11 commits into
home-assistant:devfrom
SgtBatten:patch-1
Jan 25, 2023
Merged

Add Mega Joule as valid unit of energy#86055
epenet merged 11 commits into
home-assistant:devfrom
SgtBatten:patch-1

Conversation

@SgtBatten
Copy link
Copy Markdown
Contributor

@SgtBatten SgtBatten commented Jan 17, 2023 •
edited by emontnemery
Loading

Proposed change

This Pull request adds Mega Joules as a valid energy type. In Australia our gas usage is billed in MJ despite our meters being in Cubic meters. This change provides us more flexibility in configuring our energy sensors.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
  • Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

@SgtBatten SgtBatten requested review from a team and epenet as code owners January 17, 2023 02:07
@home-assistant
Copy link
Copy Markdown
Contributor

home-assistant Bot commented Jan 17, 2023

Hey there @home-assistant/core, mind taking a look at this pull request as it has been labeled with an integration (energy) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of energy can trigger bot actions by commenting:

  • @home-assistant close Closes the issue.
  • @home-assistant rename Awesome new title Change the title of the issue.
  • @home-assistant reopen Reopen the issue.
  • @home-assistant unassign energy Removes the current integration label and assignees on the issue, add the integration domain after the command.

@SgtBatten
Copy link
Copy Markdown
Contributor Author

SgtBatten commented Jan 17, 2023

I will be looking through the dev checklist and documentation now but wanted to see if I am on the right track with the changes so far.

@Roving-Ronin Roving-Ronin mentioned this pull request Jan 17, 2023
Closed
5 tasks
@Roving-Ronin Roving-Ronin mentioned this pull request Jan 17, 2023
Closed
19 tasks
@SgtBatten
Merge pull request #1 from Roving-Ronin/patch-1
0f9ffec 
Add Mega Joule as a Unit of Measurement to Energy
@epenet
Copy link
Copy Markdown
Contributor

epenet commented Jan 17, 2023

Please take a look at #85964
This probably also needs an architecture discussion.

@frenck frenck marked this pull request as draft January 17, 2023 07:18
@frenck
Copy link
Copy Markdown
Member

frenck commented Jan 17, 2023

Indeed, marking this PR as draft. These are part of our entity models and need an architectural proposal/discussions and approval prior to implementation.

../Frenck

@emontnemery
Copy link
Copy Markdown
Contributor

emontnemery commented Jan 17, 2023

Discussion: home-assistant/architecture#859

@Roving-Ronin
Copy link
Copy Markdown
Contributor

Roving-Ronin commented Jan 17, 2023

@SgtBatten thanks for merging. Apologies on stuffing it up, as that was my original intent to add to your PR (hence my tagging), as I noticed that you had missed updating homeassistant/components/sensor/const.py and updating the doco to add suppport/mention of MJ in those files.

@jbouwh
Copy link
Copy Markdown
Contributor

jbouwh commented Jan 19, 2023

This PR will need a docs PR for sensor as well, here GJ is missing to:
https://www.home-assistant.io/integrations/sensor/

@epenet
Copy link
Copy Markdown
Contributor

epenet commented Jan 19, 2023

For reference, needs adjustments in:

  • SensorDeviceClass docstring
  • NumberDeviceClass docstring
  • energy integration
  • unit_conversion util
  • dev docs
  • user docs

This was referenced Jan 20, 2023
Closed
Merged
Merged
@SgtBatten
Copy link
Copy Markdown
Contributor Author

SgtBatten commented Jan 20, 2023 •
edited by epenet
Loading

Not 100% sure on the documentation side of things.
I've done:
home-assistant/home-assistant.io#25862 - source/_integrations/sensor.markdown

@emontnemery emontnemery marked this pull request as ready for review January 25, 2023 10:09
@emontnemery
Copy link
Copy Markdown
Contributor

emontnemery commented Jan 25, 2023

Not 100% sure on the documentation side of things. I've done: home-assistant/home-assistant.io#25862 - source/_integrations/sensor.markdown

sorry about the jargon; "dev docs" refers to updating developers.home-assistant, "user docs" refers to updating home-assistant.io

@epenet
Copy link
Copy Markdown
Contributor

epenet commented Jan 25, 2023 •
edited
Loading

NumberDeviceClass docstring is still needed

epenet
epenet reviewed Jan 25, 2023
View reviewed changes
Comment thread homeassistant/components/sensor/const.py
"""Energy.

Unit of measurement: `Wh`, `kWh`, `MWh`, `GJ`
Unit of measurement: `Wh`, `kWh`, `MWh`, `MJ`, `GJ`
Copy link
Copy Markdown
Contributor

@epenet epenet Jan 25, 2023

Choose a reason for hiding this comment

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

The same change is needed in number platform.

emontnemery
emontnemery approved these changes Jan 25, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@emontnemery emontnemery left a comment

Choose a reason for hiding this comment

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

Thanks, @SgtBatten 👍

@epenet epenet merged commit 0ccab19 into home-assistant:dev Jan 25, 2023
@SgtBatten
Copy link
Copy Markdown
Contributor Author

SgtBatten commented Jan 26, 2023

Sorry i was so slow, thanks for resolving for me

@SgtBatten SgtBatten deleted the patch-1 branch January 26, 2023 07:45
@github-actions github-actions Bot locked and limited conversation to collaborators Jan 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@epenet epenet epenet approved these changes

@emontnemery emontnemery emontnemery approved these changes

Assignees

No one assigned

Labels

cla-signed core integration: energy new-feature Quality Scale: internal small-pr PRs with less than 30 lines.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@SgtBatten @epenet @frenck @emontnemery @Roving-Ronin @jbouwh