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

Add a global device to keep track of some helpful data. #328

Merged
merged 2 commits into from
Oct 31, 2024

Conversation

Lash-L
Copy link
Contributor

@Lash-L Lash-L commented Oct 19, 2024

Finally got around to having some time to do this
image

@agittins
Copy link
Owner

This looks great!

What do you think about replacing "Device count" with two sensors: "Visible Devices" and "Total Devices", and using the method used in config_flow.py to count them (ie, visible ones have a recent stamp)? If that's a go-er then you might want to refactor that a bit and put the common part into the coordinator (unless I did already).

Perhaps they should all be in a "Diagnostics" section rather than "Sensors"... I envisage we would end up with the map and other possible sensors in the main "Sensors" category, so maybe classing these ones as diagnostic to start with would be good. I assume it's a property on the sensor that defines it.

Hehe now I'm thinking about whether the calibration stuff would be better to integrate into this page, but maybe that's better left for a later PR 😅

@Lash-L
Copy link
Contributor Author

Lash-L commented Oct 20, 2024

Update:
image

What do you think about replacing "Device count" with two sensors: "Visible Devices" and "Total Devices", and using the method used in config_flow.py to count them (ie, visible ones have a recent stamp)? If that's a go-er then you might want to refactor that a bit and put the common part into the coordinator (unless I did already).

I also used your active scanner() function - let me know what you think about that

Copy link
Owner

@agittins agittins left a comment

Choose a reason for hiding this comment

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

Looks great! 🚀

@agittins agittins merged commit f6885b9 into agittins:main Oct 31, 2024
6 checks passed
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

Successfully merging this pull request may close these issues.

2 participants