Add modbus min/max values#86131
Conversation
|
Hey there @adamchengtkc, @janiversen, @vzahradnik, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
janiversen
left a comment
There was a problem hiding this comment.
Documentation and tests are missing.
|
While drinking coffee, I was thinking about your problem....it seems to me you do not want a min_value but zero_supression which is quite a different functionality, and something that would make a lot of sense to add. In case do not know, zero_supression with X means if -X < value < X, X becomes 0 |
|
OK.. that was my original impl ! Will do this in different pr and remove thresholds so there are just min/max value |
|
So we now have: |
janiversen
left a comment
There was a problem hiding this comment.
Mainly just a naming problem.
| CONF_TARGET_TEMP, | ||
| CONF_VERIFY, | ||
| CONF_WRITE_TYPE, | ||
| CONF_ZERO_CLAMP_VALUE, |
There was a problem hiding this comment.
This is a strange name, the industry uses “zero_supress”, please change.
|
With the name changed and documentation added (green light from CI) this PR will get my approval. |
janiversen
left a comment
There was a problem hiding this comment.
Approved, thanks.
I cannot hit the approve button or merge, before the document PR is available. Remember to base the document PR on “next” (since this is a new feature).
Proposed change
Add optional min_value (with optional min_value_threashold) and max_value (with optional max_value_threashold)
Type of change
Additional information
if min_value is set:
If value < min_value_threshold, return min_value
Note: min_value_threshold defaults to min_value
if max_value is set:
If value > max_value_threshold, return max_value
Note: max_value_threshold defaults to max_value
Checklist
black --fast homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.To help with the load of incoming pull requests: