Skip to content

add percentage (DPT_Scaling) KNX sensors#8168

Merged
balloob merged 4 commits into
home-assistant:devfrom
tiktok7:support-knx-percentage-sensor
Jun 27, 2017
Merged

add percentage (DPT_Scaling) KNX sensors#8168
balloob merged 4 commits into
home-assistant:devfrom
tiktok7:support-knx-percentage-sensor

Conversation

@tiktok7
Copy link
Copy Markdown
Contributor

@tiktok7 tiktok7 commented Jun 23, 2017

  1. moved basic functionality to KNXSensorBaseClass instead of
    KNXSensorFloatClass
  2. all derived classes just override the "convert" method to handle different data formats
  3. added "if" clause in setup for a "percentage" sensor type and added KNXSensorDPTScalingClass with DPT_Scaling conversion (raw: 0 - 255 = display: 0 - 100)

Description:

This change is intended to provide support for additional data items on devices such as thermostats/covers that use the scaled percentage data format (1 byte). The existing sensors use the 2 byte float data format.

Related issue (if applicable): fixes #

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#2876

Example entry for configuration.yaml (if applicable):

 - platform: knx
   name: percent
   type: percentage
   address: 1/0/5

Checklist:

Note: there are no new dependencies and no new files

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

1. moved basic functionality to KNXSensorBaseClass instead of
KNXSensorFloatClass
2. added "if" clause in setup for a "percentage" sensor type and added KNXSensorDPTScalingClass
@mention-bot
Copy link
Copy Markdown

@tiktok7, thanks for your PR! By analyzing the history of the files in this pull request, we identified @daBONDi, @fabaff and @balloob to be potential reviewers.

Updated convert method base sensor class to avoid lint warning
(R201 - Method could be a function)
@ghost
Copy link
Copy Markdown

ghost commented Jun 23, 2017

👍

Copy link
Copy Markdown
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a PLATFORM_SCHEMA based on voluptuous that validates CONF_TYPE is one of the approved values.

Example from KNX light: https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/light/knx.py#L20-L24

@tiktok7
Copy link
Copy Markdown
Contributor Author

tiktok7 commented Jun 24, 2017

No problem. Thanks for the quick feedback.

1. added SCHEMA extension for defined keywords
2. moved fixed data for internal settings out of sensor logic
3. moved everything into standard KNXSensor object
4. added parsing of extra config parameters in __init__
@tiktok7
Copy link
Copy Markdown
Contributor Author

tiktok7 commented Jun 25, 2017

@balloob : that's the best I could do

@balloob balloob merged commit 88b9503 into home-assistant:dev Jun 27, 2017
@balloob
Copy link
Copy Markdown
Member

balloob commented Jun 27, 2017

Thanks!

@balloob balloob mentioned this pull request Jul 1, 2017
@tiktok7 tiktok7 deleted the support-knx-percentage-sensor branch July 1, 2017 11:52
@tiktok7
Copy link
Copy Markdown
Contributor Author

tiktok7 commented Jul 1, 2017

@daBONDi @Julius2342 @open-homeautomation : I should have thought of asking earlier but you three seem to have made the biggest changes on the knx platform to date. I had to make a larger than expected change to this PR to add in a PLATFORM_SCHEMA so it may have side-effects on other knx sensors but don't have enough devices to test all types. Do you think you could check your devices for any issues for the lux/speed/temp sensors on the latest dev branch?

@ghost
Copy link
Copy Markdown

ghost commented Jul 1, 2017

Temp sensors here are working fine. I don't have any Lux/speed sensors, but I would not expect problems.

dethpickle pushed a commit to dethpickle/home-assistant that referenced this pull request Aug 18, 2017
* add percentage (DPT_Scaling) KNX sensors

1. moved basic functionality to KNXSensorBaseClass instead of
KNXSensorFloatClass
2. added "if" clause in setup for a "percentage" sensor type and added KNXSensorDPTScalingClass

* support-knx-percentage-sensor: lint correction

Updated convert method base sensor class to avoid lint warning
(R201 - Method could be a function)

* added PLATFORM_SCHEMA for configuration

1. added SCHEMA extension for defined keywords
2. moved fixed data for internal settings out of sensor logic
3. moved everything into standard KNXSensor object
4. added parsing of extra config parameters in __init__

* correct lint errors on support-knx-percentage-sensor
@home-assistant home-assistant locked and limited conversation to collaborators Oct 20, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants