gillesvs / librelink

Librelink integration for Home Assistant
MIT License
21 stars 6 forks source link

Refactoring changes #15

Open kubasaw opened 4 months ago

kubasaw commented 4 months ago

This is quite huge PR with a lot of changes (I think, breaking ones):

This is just a draft PR. I've got plans for more fixes, especially regarding handling multiple tracked persons. However, I can't say for certain when those updates will be ready due to the limited time I can devote to it.

I'd love to hear your thoughts and feedback.

gillesvs commented 4 months ago

This is quite huge PR with a lot of changes (I think, breaking ones):

  • Code refactor: I've rewritten a substantial portion of our integration to streamline the maintenance process. By implementing a clearer hierarchy of class inheritance, we can now manage and extend our code more efficiently.
  • Sensor Enhancements: I've adjusted the DeviceClass for sensors to ensure they display correctly on the Home Assistant frontend. This should provide users with more accurate and informative data representation.
  • Expiration Timestamp Sensor: I've introduced a new sensor that gives a glance, when sensor will expire. For me, it is more informative than looking at application timestamp or its age.
  • Improved Low/High Sensor Functionality: Fixed an issue where Low/High are always set as OK. When I was writing the remaining parts of the code, I had a low sugar level and I was not getting the appropriate sensor state: LibreLink API always returned False for the isHigh and isLow fields 😆

This is just a draft PR. I've got plans for more fixes, especially regarding handling multiple tracked persons. However, I can't say for certain when those updates will be ready due to the limited time I can devote to it.

I'd love to hear your thoughts and feedback.

Hi Kuba,

Thanks for your contribution. I will have a look at it asap. For Low/high binary sensors, my understanding is that they only activate when you see high or low on the app (which mean under 40 or over 400). I agree it is not very useful as hopefully you do not go to these extremes often.

gillesvs commented 4 months ago

Hello @kubasaw ,

You are clearly more advanced than myself (this is my first program ;-)) but I understand than this update will "break" existing sensors. Don't you think that it is better to wait for your complete refactoring and then propose to the community a major version which will lead to sensor changes and therefore imply regressions which will have to be handled by everyone. I understand that your code is much cleaner, but some people may think that you do not need to repair what is not broken (:-)) What is your suggestion?

kubasaw commented 4 months ago

Hello,

I've created this PR as a draft, and it probably doesn't make much sense to merge it at the moment. I mainly wanted to signal that I'm working on something, and if needed, my ongoing work can be utilized.

I'll aim to finish the majority of the changes this month. I'm not entirely certain about some aspects, so if I find myself unsure about what to do next, I'll reach out here with my questions.

So if this draft can just hang around here, that would be great. Once it reaches a reasonable level of 'mergeability', I'll also think about how to sensibly migrate previous configurations.

K

gillesvs commented 4 months ago

Hello,

I've created this PR as a draft, and it probably doesn't make much sense to merge it at the moment. I mainly wanted to signal that I'm working on something, and if needed, my ongoing work can be utilized.

I'll aim to finish the majority of the changes this month. I'm not entirely certain about some aspects, so if I find myself unsure about what to do next, I'll reach out here with my questions.

So if this draft can just hang around here, that would be great. Once it reaches a reasonable level of 'mergeability', I'll also think about how to sensibly migrate previous configurations.

K

Hello, Yes please do not hesitate if you have questions. For the moment, it seems that there is no pb for users. Thanks again for your refactoring, it helps me understand better ways to structure things.