cpyarger / Home-Assistant-Addons

A hass.io addon for a software defined radio tuned to listen for Utility Meter RF transmissions and republish the data via Home Assistant's API
MIT License
9 stars 4 forks source link

Add m³ units. Water as default for r900. Symbol length to reduce cpu usage. Msg type input user friendly. #26

Closed JohnnyPicnic closed 9 months ago

JohnnyPicnic commented 1 year ago

I've added options for metric users (m³). Made the msg type input more user friendly. Added symbol length to lower cpu usage. Added water device class to r900 by default.

pasyn commented 1 year ago

Thank you for contributing! I am working on reviewing your PR. A few things so far:

JohnnyPicnic commented 1 year ago
Can you please explain why msgType should change to str? This will change the config page entry from a drop down box to a text box that is no pre-populated. How will this make input more user friendly?

User can only select one of the pre-populated options. Like in my case they would be out of luck if you didn't give them the exact combination they needed or wanted. Would be impractical to list all the possible options. You could pre fill the list with a default so users know the correct formatting.

I am okay with adding m³ as a unit of measure for water meters but we do not need to change the default.

Ok but the metric system is still better. :stuck_out_tongue_closed_eyes:

R900 changes look good.

:thumbsup:

Lowering the symbol length lowers the sample rate which in turn should lower CPU usage right? Does it make a noticeable difference? Is there any downside to lowering the symbol length?

I think depending on the type of message being read it could cause some of them to be missed. When I was using the other add-on my cpu usage was around 50%. I was able to get good result with the lowest setting of 8 dropping my cpu to 5%.

pasyn commented 1 year ago

User can only select one of the pre-populated options. Like in my case they would be out of luck if you didn't give them the exact combination they needed or wanted. Would be impractical to list all the possible options. You could pre fill the list with a default so users know the correct formatting.

This is possible but what benefit does it give over selecting "all" and filtering by ID? From what I see, the results would be the same. In theory we could remove the type filter from the config and change it to "all" in the code.

Ok but the metric system is still better.

LOL true.

I think depending on the type of message being read it could cause some of them to be missed. When I was using the other add-on my cpu usage was around 50%. I was able to get good result with the lowest setting of 8 dropping my cpu to 5%.

Per the RTLAMR wiki there may be issues caused by going over the default value of 72 so lets cap it at 72 and allow for lower values down to 8 if needed.

JohnnyPicnic commented 1 year ago

I believe it is much more efficient to only select the types of messages needed once they are known. Multi Protocol Decoding

I don't see any other way of making all the types available without a string input. I guess you could do booleans for each one?

KanyonKris commented 1 year ago

There are only 3 meter types so I suggest changing the UI structure to:

options:
  debug: False
  rtltcpdebug: False
  duration: 15
  pause_time: 30
  electric_id: ""
  electric_msg_type: scm
  electric_unit_of_measurement: kWh
  electric_multiplier: 1.0
  gas_id: ""
  gas_msg_type: scm+
  gas_unit_of_measurement: ft³
  gas_multiplier: 1.0
  water_id: ""
  water_msg_type: idm
  water_unit_of_measurement: gal
  water_multiplier: 1.0

schema:
  debug: bool
  rtltcpdebug: bool
  duration: int
  pause_time: int
  electric_id: str?
  electric_msg_type: list(scm|scm+|idm|netidm|r900|r900bcd|all)
  electric_unit_of_measurement: list(Wh|kWh|MWh)
  electric_multiplier: float
  gas_id: str?
  gas_msg_type: list(scm|scm+|idm|netidm|r900|r900bcd|all)
  gas_unit_of_measurement: list(ft³|m³)
  gas_multiplier: float
  water_id: str?
  water_msg_type: list(scm|scm+|idm|netidm|r900|r900bcd|all)
  water_unit_of_measurement: list(gal|l)
  water_multiplier: float
KanyonKris commented 1 year ago

I believe it is much more efficient to only select the types of messages needed once they are known. Multi Protocol Decoding

Note the warnings from the author of rtlamr:

-msgtype=all includes the following protocols: scm, scm+, idm, r900 and is intended to be used for discovery.

r900 and r900bcd are fairly resource heavy and should not be used concurrently with other protocols on low-power systems like RPi's.