Skip to content

Refactor, bug fixes, and improvements - #1

Merged
FutureTense merged 4 commits into
FutureTense:mainfrom
raman325:refactor_improvements_and_bug_fixes
Dec 9, 2020
Merged

Refactor, bug fixes, and improvements#1
FutureTense merged 4 commits into
FutureTense:mainfrom
raman325:refactor_improvements_and_bug_fixes

Conversation

@raman325

@raman325 raman325 commented Dec 7, 2020

Copy link
Copy Markdown
Collaborator

Proposed change

Hi there, I started down this path because:

  1. I was having trouble getting the integration to work
  2. I wanted to figure out if there was a clean way to remove entities on unload since a lot end up getting left over.

I didn't make it very far on 2 but I did make good progress on 1 and in the middle of it added a bunch of scope creep. I apologize for the size of the PR, but I thought it would be a good way to make this more maintainable going forward. If you see some changes you'd like me to make, or if there are parts of the refactor you would like me to revert, let me know. I haven't changed much of the actual logic as you'll see in the notes below since it all comes from the package templates and the sensors.

NOTE: I have tested this and as far as I can tell, everything is working as expected, but my integration hasn't been working since before my code changes (specifically, the PIN gets set but PIN Status shows as Connecting forever), so it would be great if someone else could test it

New Features

  • Packages folder gets cleaned up on unload

Changes

  • switch to using relative path for CONF_PATH to reduce changes of error (existing config entries will be updated automatically)

Bug Fixes

  • Switch _generate_package from async to sync because it does I/O on the filesystem
  • fix output file not being opened with context manager (so never closed)
  • fix sensor creation loop to only create necessary sensors instead of sensors for all code slots
  • fix CodeSlotsData sensor (now a DataUpdateCoordinator) logic for OZW integrations

Code Cleanup/Improvements

  • move CodeSlotsData logic into DataUpdateCoordinator since that's what it is being used for
  • reduce polling frequency significantly by only polling any time a slot is set or cleared with a backup hourly polling in case updates are made outside of KeyMaster (this will significantly improve battery life over current implementation with the same level of accuracy)
  • Update config entry using update method instead of directly manipulating attributes
  • use slugify when fixing CONF_NAME
  • use os.path.join to create paths for better OS compatibility
  • switch to using f-strings
  • only attempt cleanup of packages directory if it exists
  • always rely on entry.data so that options is only used to capture options updates
  • simplify code and reduced repeat logic by moving things into common functions
  • raise Exceptions in services when there is a failure (this will be shown in UI)
  • use more list comprehensions
  • switch while loops to for loops
  • added typing
  • add venv to .gitignore
  • add requirements_dev.txt
  • improve debug messaging

TODO (in future PRs)

  • On options update, clean up no longer used entities
  • On removal, cleanup all entities that were created from packages

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (which adds functionality)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:

@raman325
raman325 force-pushed the refactor_improvements_and_bug_fixes branch from 7fcf419 to b688839 Compare December 7, 2020 22:28
Comment thread custom_components/keymaster/const.py Outdated
@firstof9

firstof9 commented Dec 7, 2020

Copy link
Copy Markdown
Collaborator

These changes look good to me. I like the changes auto updating the code slots data myself.
Guess I won't have to do the DataCoordinator implimentation myself either 🎉

@firstof9
firstof9 requested a review from FutureTense December 7, 2020 22:58
@firstof9 firstof9 added enhancement New feature or request bugfix Fixes a bug labels Dec 7, 2020
Comment thread custom_components/keymaster/__init__.py Outdated

@firstof9 firstof9 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Changes look good now 👍

New Features
- cleanup packages folder on entry removal

Changes
- switch to using relative path for CONF_PATH to reduce changes of error (existing config entries will be updated automatically)

Bug Fixes
- Switch _generate_package from async to sync because it does I/O on the filesystem
- fix output file not being opened with context manager (so never closed)
- fix sensor creation loop to only create necessary sensors instead of sensors for all code slots
- fix CodeSlotsData sensor (now a DataUpdateCoordinator) logic for OZW integrations

Code Cleanup/Improvements
- move CodeSlotsData logic into DataUpdateCoordinator since that's what it is being used for
- reduce polling frequency significantly by only polling any time a slot is set or cleared with a backup hourly polling in case updates are made outside of KeyMaster (this will significantly improve battery life over current implementation with the same level of accuracy)
- Update config entry using update method instead of directly manipulating attributes
- use slugify when fixing CONF_NAME
- use os.path.join to create paths for better OS compatibility
- switch to using f-strings
- only attempt cleanup of packages directory if it exists
- always rely on entry.data so that options is only used to capture options updates
- simplify code and reduced repeat logic by moving things into common functions
- raise Exceptions in services when there is a failure (this will be shown in UI)
- use more list comprehensions
- switch while loops to for loops
- added typing
- gitignore venv
- improve debug messaging
@raman325
raman325 force-pushed the refactor_improvements_and_bug_fixes branch from e5cd1a9 to cdef9b1 Compare December 9, 2020 17:55
@FutureTense
FutureTense merged commit 775250e into FutureTense:main Dec 9, 2020
github-actions Bot pushed a commit that referenced this pull request Dec 9, 2020
* refactor, bug fixes, and improvements

New Features
- cleanup packages folder on entry removal

Changes
- switch to using relative path for CONF_PATH to reduce changes of error (existing config entries will be updated automatically)

Bug Fixes
- Switch _generate_package from async to sync because it does I/O on the filesystem
- fix output file not being opened with context manager (so never closed)
- fix sensor creation loop to only create necessary sensors instead of sensors for all code slots
- fix CodeSlotsData sensor (now a DataUpdateCoordinator) logic for OZW integrations

Code Cleanup/Improvements
- move CodeSlotsData logic into DataUpdateCoordinator since that's what it is being used for
- reduce polling frequency significantly by only polling any time a slot is set or cleared with a backup hourly polling in case updates are made outside of KeyMaster (this will significantly improve battery life over current implementation with the same level of accuracy)
- Update config entry using update method instead of directly manipulating attributes
- use slugify when fixing CONF_NAME
- use os.path.join to create paths for better OS compatibility
- switch to using f-strings
- only attempt cleanup of packages directory if it exists
- always rely on entry.data so that options is only used to capture options updates
- simplify code and reduced repeat logic by moving things into common functions
- raise Exceptions in services when there is a failure (this will be shown in UI)
- use more list comprehensions
- switch while loops to for loops
- added typing
- gitignore venv
- improve debug messaging

* fix typo

* fix clear code logic and make it less ambiguous

* styling consistency
@raman325
raman325 deleted the refactor_improvements_and_bug_fixes branch December 9, 2020 18:06
@tykeal tykeal mentioned this pull request Jun 12, 2026
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Fixes a bug enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants