tomaae / homeassistant-mikrotik_router

Mikrotik router integration for Home Assistant
Apache License 2.0
301 stars 50 forks source link

[Bug] kilo prefix should be lower case #211

Closed mvdwetering closed 2 years ago

mvdwetering commented 2 years ago

Describe the issue

The units that have the kiloprefix should be in lowercase "k" instead of uppercase "K".

How to reproduce the issue

In the integration configuration settings or values of sensors, there is "Kbps" or "KB/s" instead of "kbps" or "kB/s"

Expected behavior

Lowercase "k" instead of uppercase "K"

Screenshots

Software versions

Diagnostics data

Traceback/Error logs

No errors

Additional context

tomaae commented 2 years ago

Thats actually a good point worth researching. Not sure if mikrotik returns binary or decimal using API. k = binary kilobyte K = decimal kilobyte

decimal is normally used to measure bandwidth, so uppercase should be correct. But in case of API, not sure.

mvdwetering commented 2 years ago

Ah, I did not know about the big "K", I only knew "k" and "Ki"

Looking into it a bit more and what I gather from Wikipedia kilo and binary prefix

k = decimal Ki (for kibi) = binary (apparently K was used in the past as it is mentioned as Legacy, guess it was renamed to avoid confusion like I had :) )

Home Assistant seems to use/prefer(?) the "Ki" variant as there is a DATA_KIBIBYTES: Final = "KiB" and DATA_RATE_KIBIBYTES_PER_SECOND: Final = "KiB/s"

tomaae commented 2 years ago

yea, decimal is still used in bandwidth. but storage is more commonly in binary now. winbox uses lowercase for rate, uppercase for total. so something is wrong there. edit: actually, it could make sense for data transfered, since thats not bandwidth.