Change pressure unit of measurement from mbar to hPa in Netatmo integration

[ljungqvist]

Proposed change

The device_class of the pressure in the Netatmo integration changed from PRESSURE to ATMOSPHERIC_PRESSURE.

This means the unit of measurement will be automatically converted from mbar, which is used by the Netatmo API to hPa which is recommended by the World Meteorological Organization for reporting atmospheric pressure, if the user's unit system is metric, and to inHg if it's US customary.

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

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:

• Documentation added/updated for www.home-assistant.io

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:

• I have reviewed two other open pull requests in this repository.

[home-assistant]

Hi ljungqvist

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

[home-assistant]

Hey there @cgtobi, mind taking a look at this pull request as it has been labeled with an integration (netatmo) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of netatmo 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 netatmo Removes the current integration label and assignees on the issue, add the integration domain after the command.

[frenck]

This is a breaking change.

Is that needed? As in, if you like prefer hPa, you can change the unit for your entity to your liking.

[epenet]

I think the device_class should be adjusted instead to use SensorDeviceClass.ATMOSPHERIC_PRESSURE

See

core/homeassistant/util/unit_system.py

Lines 261 to 266 in 74096b8

	# Force atmospheric pressures to hPa
	**{
	("atmospheric_pressure", unit): UnitOfPressure.HPA
	for unit in UnitOfPressure
	if unit != UnitOfPressure.HPA
	},

and

core/homeassistant/util/unit_system.py

Lines 302 to 307 in 74096b8

	# Force atmospheric pressures to inHg
	**{
	("atmospheric_pressure", unit): UnitOfPressure.INHG
	for unit in UnitOfPressure
	if unit != UnitOfPressure.INHG
	},

[epenet]

This is a breaking change.

Because there is a device_class and the unit is compatible, it shouldn't be a breaking change.
However, I think adjusting the device_class is better if it always refers to atmospheric pressure.

[ljungqvist]

This is a breaking change.

Is that needed? As in, if you like prefer hPa, you can change the unit for your entity to your liking.

The default should be hPa, as it is the correct unit for atmospheric pressure.

[frenck]

? That is what the description already said. IMHO not enough to justify a breaking change. See also comment from epenwt above.

[epenet]

Changing the device_class is NOT a breaking change (existing sensors will keep using the old unit, new sensors will get the new unit).

Regarding the native_unit_of_measurement, the netatmo documentation specifies mbar in https://dev.netatmo.com/apidocumentation/weather so I think it's better to keep it as it is and avoid any chance of a breaking change.

[emontnemery]

@frenck, @epenet this should not be a breaking change: thanks to #83228 a new unit, which is compatible with the old unit stored in the entity registry will be converted to the old unit.

[frenck]

Cool! Still, see the last comment by @epenet, it is communicated as mbar by the API itself as well. We should match upstream. We should set a suggested unit if we think it should differ.

But that said, if it is a standard we want to have, we should take care of that system-wide (like preferred units as planned originally) and not a one-off.

That said, it is still a breaking change for external systems (like InfluxDB or any other recording system). -> No longer true indeed 👍

[epenet]

Note: whatever the integration says, it will be converted to hPa for "atmospheric_pressure"

[emontnemery]

I reverted the change of unit, which is not needed, and also updated the PR description accordingly. Thanks for the PR @ljungqvist 👍

[frenck]

Thanks, @ljungqvist 👍

../Frenck