search

mysensors / MySensors

MySensors library and examples
https://www.mysensors.org
1.32k stars 895 forks source link

Fixing V_FLOW data type measurement unit #1539

Closed paolo-rendano closed 1 year ago

paolo-rendano commented 1 year ago

In Home Assistant core component integration we are discussing [here] (https://github.com/home-assistant/core/pull/85835/files/2fe27bec4cc0bec419026f0ca823cfe0d85a58a0) about the evolution of the V_FLOW data type which is currently defined here with a clearly wrong measurement unit (meters).

This is as by MySensors definition and also confirmed by the usage here as a volumetric flow which is defined by physics [ref here] (https://en.wikipedia.org/wiki/Volumetric_flow_rate) to be expressed in m3 or ft3 per time unit (a volume of a fluid moving for a time).

We would like to change accordingly the description and the related example (switching to m3/h). Eventually, I can take care of this.

Any thoughts here?

mfalkvidd commented 1 year ago

m3/h sounds good to me.

paolo-rendano commented 1 year ago

m3/h sounds good to me.

ok good, so if no other will make any exception to this in the next days, i'll add a pull request for changing this, and once i'll have access to CMS will change this in the page accordingly. I'll move on the same way on the home assistant pull request that i've done.

paolo-rendano commented 1 year ago

I've just made the pull request. I'll need to change the CMS page for this.

The pages that i need to change is https://www.mysensors.org/download/serial_api_20 (which reports wrong V_FLOW measurement unit (meter...) and this should be switched to m3/h

paolo-rendano commented 1 year ago

@mfalkvidd for this kind of change that will imply some changes in the home assistant release, I guess we'll need a new release of mysensors here, right?

mfalkvidd commented 1 year ago

Sorry, I don't know. I am not aware of how the Home Assistant release process works, nor how it could affect MySensors. I have trouble seeing how changing a code comment would trigger a new release; it's not like we're making breaking changes to an API.

paolo-rendano commented 1 year ago

Sorry, I don't know. I am not aware of how the Home Assistant release process works, nor how it could affect MySensors. I have trouble seeing how changing a code comment would trigger a new release; it's not like we're making breaking changes to an API.

Ok, there is anyway some bidirectional relationship between Home Assistant integration and MySensors. I understand here we have only a change in the example and in the documentation, so It could be ok to not make a new release. I'm not aware of the in place practices between MySensors and HAss, but if changing MySensors doc and simply link the development branch for the new example, this is fine also for me. Let's finalize this way (as soon as jenkins is ok with build)

MartinHjelmare commented 1 year ago

As far as Home Assistant and its MySensors integration is concerned, there's no need for a new release of MySensors. If the documentation and example for this message type is updated that's enough for Home Assistant to rely on that when setting default unit of measurement for this sensor type.

paolo-rendano commented 1 year ago

@mfalkvidd this is completed, can you merge this? So that i'll move on things on HA as well and ask merge also the PR on their side.

thanks Paolo

MartinHjelmare commented 1 year ago

I think you meant to tag Mikael.

mfalkvidd commented 1 year ago

https://github.com/mysensors/MySensors/pull/1541 merged

paolo-rendano commented 1 year ago

I've just made the pull request. I'll need to change the CMS page for this.

The pages that i need to change is https://www.mysensors.org/download/serial_api_20 (which reports wrong V_FLOW measurement unit (meter...) and this should be switched to m3/h

@mfalkvidd who can fix and publish the cms page? I think that also for the example in the Water sensor, as discussed before, this should be switched to the dev branch so to show the right example without need for a release