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.

Sign up for GitHub

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

Jump to bottom

Error when calling get_basic_device_stats for newly installed thermostat #215

Open
jmozmoz opened this issue Mar 30, 2024 · 3 comments · May be fixed by #216
Open

Error when calling get_basic_device_stats for newly installed thermostat #215

jmozmoz opened this issue Mar 30, 2024 · 3 comments · May be fixed by #216

Comments

@jmozmoz
Copy link

jmozmoz commented Mar 30, 2024

When calling get_basic_device_stats() for a newly installed thermostat which does not have temperature data recorded for 24 h, the method throws an error:

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
Cell In[10], line 3
      1 d = fh.get_homeautomation_devices()[0]
      2 for d in fh.get_homeautomation_devices():
----> 3     data = d.get_basic_device_stats()
      4     print(data['temperature']['datatime'], data['temperature']['count'])
      5     date = data['temperature']['datatime'] - timedelta(seconds=data['temperature']['grid'])*np.arange(data['temperature']['count']) #95, -1, -1)

File ~/heizung/lib/python3.9/site-packages/fritzconnection/lib/fritzhomeauto.py:374, in HomeAutomationDevice.get_basic_device_stats(self)
    348 """
    349 Returns a dictionary of device statistics. The content depends on
    350 the actors supported by a device. The keys can be:
   (...)
    371 properties are much(!) faster.
    372 """
    373 response = self.call_http("getbasicdevicestats")
--> 374 return self.extract_basicdevicestats_response(response)

File ~/heizung/lib/python3.9/site-packages/fritzconnection/lib/fritzhomeauto.py:398, in HomeAutomationDevice.extract_basicdevicestats_response(response)
    396             value = datetime.datetime.fromtimestamp(value)  # type: ignore
    397         content[key] = value
--> 398     content["data"] = list(map(int, stats.text.split(",")))  # type: ignore
    399     elements[element.tag] = content
    400 return elements

ValueError: invalid literal for int() with base 10: '-'

The reason is, that the returned XML does not contain numbers but "-":

d.call_http('getbasicdevicestats')
{'content-type': 'text/xml',
 'encoding': 'utf-8',
 'content': '<devicestats><temperature><stats count="96" grid="900" datatime="1711822481">190,195,195,190,185,190,195,195,195,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-,-</stats></temperature></devicestats>\n'}
@jmozmoz jmozmoz linked a pull request Mar 30, 2024 that will close this issue
Open
@kbr
Copy link
Owner

kbr commented Mar 31, 2024

Thank you for reporting the issue and the according pull request.

Beside this I'm not sure whether the change will be physically correct. To understand this it is important to know how the sensor works. Is it okay to change the count-value according to the changed number of values which will invalidate the grid-value? I would assume that the grid-value is constant and also the number of counts do not change, but a dash in the data represents a missing datapoint. If this is the case the missing datapoint can not be ignored. Using the original dash (or None) would result in a list of data with mixed types (delegating further exception handling elsewhere in the application based on the library). To keep same types an integer representing missing (not unknown!) data could be taken like -999, which is historical used for missing temperature records. Again it would be up to applications to handle this.

Also these changes must go to the test-code and -data and also to the documentation.

(It could also be an idea to get in contact with the thermostat-vendor to get first hand information about the protocol for missing data. So far I'm just guessing about the interpretation of missing data.)

@jmozmoz
Copy link
Author

jmozmoz commented Mar 31, 2024 •
edited

The current pull request is just a first attempt to get this to work. I think, an exception in fritzconnection itself should not happen. So I think it wont make sense to try to convert everything to integers and throw a ValueError or something else, if this does not work. Also, different types in the returned list probably are hard to handle. Therefore, I reduced the list so only integers are returned.

So I guess, if there is an "official" integer value for an invalid value, this should be used for the '-'.

@kbr
Copy link
Owner

kbr commented Apr 1, 2024

I agree, the ValueError should get catched and the dash should get converted to something else. In my original test data from a sensor I have had just zeros and no dashes. A possible integer value for missing data could be -999 as by historical DWD weather data. However I would prefer None even if this results in a list of mixed types. This list can assumed as "raw data".

On top of this the library may get an additional method to provide a list based on the "raw data" with tuples of timestamps and values (which are integers and not None). This would be a datastructure that can get easily adapted for plotting or i.e. as input for pandas series. Optional it could also provide the missing data as None or another user defined default value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

only use integers in data and set count accrodingly jmozmoz/fritzconnection
2 participants
@kbr @jmozmoz