[systeminfo] QuantityTypes and state descriptions cleanup

[mherwege]

Signed-off-by: Mark Herwege mark.herwege@telenet.be

Changes in core UOM handling make the system uptime channel interpret the minutes as seconds when linking an Number:Time item to the channel. This is because the state description has a wrong UOM. See core issue #3183.
This PR does a cleanup of descriptions and default state descriptions to avoid this issue.

Closes #3183.

Another issue has been discovered #13709 with unit conversions in relation to this binding. I don't see an immediate reason for this in the binding. If you have ideas, please suggest and I will include in this PR.

[mherwege]

Test fail because DataAmount unit conversions do not work properly, see #3207.
The code is complete from my perspective. If unit conversions for DataAmount are fixed, this can be reviewed and merged.

[lolodomo]

This a breaking change as users will have to change their item types.

[lolodomo]

Test fail because DataAmount unit conversions do not work properly, see #3207.
The code is complete from my perspective. If unit conversions for DataAmount are fixed, this can be reviewed and merged.

What are we doing ?
Can you comment the failing integration until the conversion is fixed in core framework ? In that case, I could merge your changes.
Or should we wait for a fix in core framework ?

[lolodomo]

Code is good for me, I am just waiting for a successful compilation.

[lolodomo]

DataAmount unit conversions do not work properly, see openhab/openhab-core#3207.

Maybe (just an idea) the problem could be that we are using a unit defined by the OH core framework (Units.MEBIBYTE), not an official unit ?

Edit: I see that @J-N-K just proposed a fix in core framework.

[J-N-K]

DataAmount unit conversions do not work properly, see openhab/openhab-core#3207.

Maybe (just an idea) the problem could be that we are using a unit defined by the OH core framework (Units.MEBIBYTE), not an official unit ?

This will also fail. The fix is in openhab/openhab-core#3208. But it should be checked if the binding really delivers kB (1000 B) or kiB (1024 B), same for MB/MiB and GB/GiB and also the data transfer rates. This is quite confusing but hard to change now without breaking existing installations.

[lolodomo]

This will also fail. The fix is in openhab/openhab-core#3208. But it should be checked if the binding really delivers kB (1000 B) or kiB (1024 B), same for MB/MiB and GB/GiB and also the data transfer rates.

Good remark. Hoping @mherwege can retrieve this information.

This is quite confusing but hard to change now without breaking existing installations.

Of course, users will have to change their item types. But as this PR improve something (ability for the users to choose their displayed units), this is acceptable, no ?
By the way, @mherwege already broke something for this binding in OH3.4 so this is just something additional to clearly mentioned in the release notes.
Is it ok for you @J-N-K ?

[J-N-K]

The binding should return the correct value (probably MiB and not MB). My comment was regarding changing 1 MB to 1024 kB in core. Even if I doubt that someone really expects the 1000 instead of 1024 when using these units…

[lolodomo]

The binding should return the correct value (probably MiB and not MB). My comment was regarding changing 1 MB to 1024 kB in core. Even if I doubt that someone really expects the 1000 instead of 1024 when using these units…

Understood.
Hopefully, this was not too much used until now in bindings.
But your remark means @mherwege should also think if the best channel pattern should use MB or MiB.

For the 2 other bindings, I hope there is no mismatch between MB and MiB.

[mherwege]

So far, the binding always returned MiB (so 1024 x 1024 bytes), but the state description was MB. I kept the state description, but the methods return MiB (there is even a method in the binding (getSizeInMB) that actually returns MiB, rather than MB. All methods returning data amounts rely on this method. So the calculation is correct.
I opted to keep the default state description, but this will lead to slight changes in value. As an alternative, I could change the default state descriptions to MiB, this will then keep the values constant. I can live with whatever choice.

[mherwege]

This a breaking change as users will have to change their item types.

I think you should still be able to just keep a Number item without dimension linked to the channels with dimension. So I am not sure this makes it a breaking change.
The MB vs MiB issue is however breaking, whatever choice we make.

[mherwege]

The binding should return the correct value (probably MiB and not MB).

Makes sense, so went for that in the last commit.

[lolodomo]

LGTM

[lolodomo]

Can I merge or should I wait for the fix in core framework?

Can you prepare something for the release notes?

[mherwege]

@lolodomo Yes, I will prepare something for the release notes. It should work now as long as one does not do unit conversions on DataAmount and sticks with MiB. That is no worse then it was before, and will get better ones the core change is done.
I now also adjusted the README. Should I still push a commit in this PR, or create a separate one for this?

[lolodomo]

I now also adjusted the README. Should I still push a commit in this PR, or create a separate one for this?

Please do it here. I will wait.

[mherwege]

Where do I have to push the content for the release notes?

[lolodomo]

LGTM, thank you

[lolodomo]

Where do I have to push the content for the release notes?

Your PR in openhab-distro is fine and should be sufficient.
I don't know if Kai will ask for help when he will write the 3.4 release notes.