Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enabled ENUM sensors #7

Open
wants to merge 6 commits into
base: master
from
Open

Enabled ENUM sensors #7

wants to merge 6 commits into from

Conversation

@kremers
Copy link

kremers commented Mar 24, 2020

Signed-off-by: Martin Kremers info@martinkremers.de

Fixes #6

Signed-off-by: Martin Kremers <info@martinkremers.de>
@sfudeus

This comment has been minimized.

Copy link
Owner

sfudeus commented Mar 25, 2020

Hi Martin - thanks for your contribution.
Does your proposed patch really solve your problem? Adding the enumset as a label seems a bit awkward to me. What is the metric value in that case? Is that supposed to be the index in the enumset?
Unfortunately I have no device here which would show an enum different then index 0 currently.

Regarding the labels of the metrics, I'd sooner add a label with the name of the enum item and value 1
Example:
Instead of

homematic_current_status{ccu="hostname",device="xxxxxxxxxxxx:6",device_type="ENERGIE_METER_TRANSMITTER",mapped_name="xxxxxxxxxx:6",param_desc="['NORMAL', 'OVERFLOW', 'UNDERFLOW']",parent_device_type="HMIP-PSM"} 0.0

it would be

homematic_current_status{ccu="hostname",device="xxxxxxxxxxxx:6",device_type="ENERGIE_METER_TRANSMITTER",mapped_name="xxxxxxxxxx:6",status="NORMAL",parent_device_type="HMIP-PSM"} 1.0
homematic_current_status{ccu="hostname",device="xxxxxxxxxxxx:6",device_type="ENERGIE_METER_TRANSMITTER",mapped_name="xxxxxxxxxx:6",status="OVERFLOW",parent_device_type="HMIP-PSM"} 0.0
homematic_current_status{ccu="hostname",device="xxxxxxxxxxxx:6",device_type="ENERGIE_METER_TRANSMITTER",mapped_name="xxxxxxxxxx:6",status="UNDERFLOW",parent_device_type="HMIP-PSM"} 0.0

What do you think?

exporter.py Outdated
logging.debug("Found {}: desc: {} key: {}".format(paramType,paramDesc,paramset.get(key)))
self.processSingleValue(devAddress, devType, devParentAddress, devParentType, paramType, key, paramset.get(key), paramDesc.get('VALUE_LIST'))
else:
# ATM Unsupported like HEATING_CONTROL_HMIP.PARTY_TIME_START,

This comment has been minimized.

Copy link
@sfudeus

sfudeus Mar 25, 2020

Owner

wrong indentation

This comment has been minimized.

Copy link
@kremers
@kremers

This comment has been minimized.

Copy link
Author

kremers commented Mar 26, 2020

Does your proposed patch really solve your problem?

Yes, I am using it already with an own docker image on my rpi

Is that supposed to be the index in the enumset?

Yes

What do you think?

Having all enum values and the selected index will allow queries to be way simpler using enum/value mapping in grafana later on. One con is that it won't adapt well for changing enums, but I consider that a very rare case.

As far as I understand StateSetMetricFamily has to be used to generate your intended result?

@kremers

This comment has been minimized.

Copy link
Author

kremers commented Mar 27, 2020

Alternative Version for review: #8

@kremers

This comment has been minimized.

Copy link
Author

kremers commented Mar 30, 2020

Closed #8 in favour of the most up to date master, container your suggestions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.