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

Update on HA integration and some suggestions for the API #4

Open
rrooggiieerr opened this issue Dec 15, 2022 · 3 comments
Open

Update on HA integration and some suggestions for the API #4

rrooggiieerr opened this issue Dec 15, 2022 · 3 comments

Comments

@rrooggiieerr
Copy link

Hi! I thought you might be interested in the status of the HA integration so attached you'll find some screenshots. The pull request which I created yesterday is required to make the integration work. So when that's incorporated I can release the integration.

It's not entirely clear to me how the data groups work. Can you elaborate?

Also some thing I notices in your API which could use some fine tuning:
The function data_threshold expects the value to be in milliamperes while record_intensity_threshold in the UMmeterData object is converted to amperes.

In my opinion the keys record_capacity_threshold, record_energy_threshold and record_intensity_threshold don't really cover the purpose. I think recorded_capacity, recorded_energy and recording_treshold would be better fitting. What do you think?

Screenshot 2022-12-15 at 17 50 50

Screenshot 2022-12-15 at 17 51 01

@rrooggiieerr rrooggiieerr changed the title Update in HA integration and some suggestions for the API Update on HA integration and some suggestions for the API Dec 15, 2022
@valletw
Copy link
Owner

valletw commented Dec 19, 2022

Nice board !
For the data groups and record information, you can check here: Wiki RDTech_UM_series.
I used this wiki to format the data. The groups can store persistent data across device plug/unplug. I didn't used it, so I cannot help you on that point.

@rrooggiieerr
Copy link
Author

I like how you implemented the UMmeterInterface!

Receiving response from te socket really needs to be done as follows

result = b''
while len(result) < nb:
    result += self._socket.recv(nb)
return result

and not

return bytearray(self._socket.recv(nb))

This will not read all data

I would like to add a bit more Exception handling and also some logging. Is that ok for you?

Also I need to wrap the socket in asynio to make it work with Home Assistant

@valletw
Copy link
Owner

valletw commented Dec 24, 2022

Hi,
I pushed the interface modification (without BT support), and publish a new release.
My time will be limiting to work on the BT support for the next weeks/months. If you can make it by reusing my branch or directly from your preview PR it will be nice. I will try to follow the thread to help you and validate your modification.

I never implement asyncio on a project, so may be add the support of this first, and another PR for the BT support.

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

No branches or pull requests

2 participants