Add madvr envy integration

[iloveicedgreentea]

Proposed change

This adds an integration for the madvr envy http://madvrenvy.com

Type of change

• Dependency upgrade
• Bugfix (non-breaking change which fixes an issue)
• New integration (thank you!)
• New feature (which adds functionality to an existing integration)
• Deprecation (breaking change to happen in the future)
• 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:
• Link to documentation pull request: Madvr home-assistant.io#33401

Checklist

• The code change is tested and works locally.
• Local tests pass. Your PR cannot be merged unless tests pass
• There is no commented out code in this PR.
• I have followed the development checklist
• I have followed the perfect PR recommendations
• The code has been formatted using Ruff (ruff format homeassistant tests)
• Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

• The manifest file has all fields filled out correctly.
  Updated and included derived files by running: python3 -m script.hassfest.
• New or updated dependencies have been added to requirements_all.txt.
  Updated by running python3 -m script.gen_requirements_all.
• For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
• Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

• I have reviewed two other open pull requests in this repository.

[home-assistant]

When adding new integrations, limit included platforms to a single platform. Please reduce this PR to a single platform

[home-assistant]

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

[iloveicedgreentea]

Changes:

• Use runtime data
• Split out schema from config flow
• Add types to missing params
• Remove lingering event bus stuff
• Remove unneeded methods
• Write state after changing it
• Fix misc comments

Is it possible to maybe start with a platform that holds a state? Currently its very unclear what data you have, what you work with and that will affect how it is going to look in the end.

Not sure what you mean. The library handles receiving push data from the device. This integration reads self.madvr_client.msg_dict

Power state is read directly from the device via the library in self.madvr_client.is_on

Otherwise, it sends commands to the device. This works perfectly fine I have been testing this integration for 3 weeks now and the library itself has been used by many people for about a year now. I used most of this code in a custom component for over a year too.

[joostlek]

I think its good to take a step back at this point. How does the data work in the integration? What does the state of the data look like?

Some things don't make sense and I think we better take a step back and increase the shared knowledge before we send you in a direction that does not work. Because you are creating a MadVR client in the __init__.py, and then you are actually closing the connection the moment the remote is removed from hass (in async_will_remove_from_hass) even though this also happens when someone disabled that entity. So this would lead to unwanted side effects.

I also think that we should explore putting the whole queue mechanism into the library, as I think that is a device/connection specific detail.

[iloveicedgreentea]

I think its good to take a step back at this point. How does the data work in the integration? What does the state of the data look like?

I still really don't understand this comment. The integration literally reads a dict in the library which is written by the library when the device pushes data through the connection, like a websocket. This is how the device is designed.

Some things don't make sense and I think we better take a step back and increase the shared knowledge before we send you in a direction that does not work. Because you are creating a MadVR client in the init.py, and then you are actually closing the connection the moment the remote is removed from hass (in async_will_remove_from_hass) even though this also happens when someone disabled that entity. So this would lead to unwanted side effects.

I think what doesn't make sense is the HA dev docs. It is frustratingly hard to understand "the right way" to do something. All the guidance given in this PR is not found in the docs and I have just been following the docs to create this integration. Those functions don't have explanations like that

So are you saying to handle connection closing in async_unload_entry? You need to understand I don't have the institutional knowledge you have. I only have access to the docs. And the docs say that async_will_remove_from_hass is used to "disconnect from the server" The docs also say async_unload_entry is used to "close all connections" so which one do I use? The functions seem to do the exact same thing.

I also think that we should explore putting the whole queue mechanism into the library, as I think that is a device/connection specific detail.

This is fair I will move this to the library

[joostlek]

The integration literally reads a dict in the library which is written by the library when the device pushes data through the connection, like a websocket. This is how the device is designed

I did not know this. I could only assume and since in this case I was confused with all the things happening with tasks and stuff I just asked the quest to be sure :)

I think what doesn't make sense is the HA dev docs. It is frustratingly hard to understand "the right way" to do something. All the guidance given in this PR is not found in the docs and I have just been following the docs to create this integration. Those functions don't have explanations like that

1. The dev docs are lacking and I acknowledge that. It's something I want to work on in the near future since I also only gained this knowledge in the last year.
2. There is no right way for a lot of things as its integration dependent. I haven't seen an integration that is exactly like another one. But there are patterns to be seen.

And the docs say that async_will_remove_from_hass is used to "disconnect from the server" The docs also say async_unload_entry is used to "close all connections" so which one do I use? The functions seem to do the exact same thing.

In the past, a lot of integrations just had 1 entity which contained a lot of data. In the recent times we've seen a move towards more entities instead of more data in 1 entity, as the first one is more user friendly. But the docs haven't really grown with that. So yea, in the past it was more common to disconnect at that place, since with YAML there was no way of unloading. So yes, use async_unload_entry as that is integration scale instead of entity scale.

And please don't forget, I am not here to call you out on stuff you did wrong, I am trying to point you at the relevant things that could increase the code quality and enhancing best practices :). I am currently looking into how we can word best practices and present them as a guide to enhance the steps to create a new integration.

[iloveicedgreentea]

And please don't forget, I am not here to call you out on stuff you did wrong, I am trying to point you at the relevant things that could increase the code quality and enhancing best practices. I am currently looking into how we can word best practices and present them as a guide to enhance the steps to create a new integration.

Thats fine actually I don't mind being corrected because I want to do it the right way. I do think that, with complex systems, there is a "right way" and I would love for HA to be more opinionated with that. HA should prescribe ways to do this with patterns rather than documenting functions. For example, in my case I am using a push method, so there should be one way to do this. Having documentation that points to existing code bases which are exemplars of this would be a good start. This is the feedback I see anyway, that people need to dive into core and just blindly copy existing integrations.

e.g in the getting started docs, have a section with common patterns:

Is your library pushing data to the integration? Here is how you set that up and here is an exemplar integration

Anyway, I pushed a change which moves more things into the coordinator and redid the unload stuff. Hopefully that is right. I am working on moving the queue stuff into the library

[iloveicedgreentea]

I did not know this. I could only assume and since in this case I was confused with all the things happening with tasks and stuff I just asked the quest to be sure :)

I mentioned it before but I can add this to comments in the code if that will help. The tasks are used to process things in real time

1. Handle queue (moved to library) this processes commands in real time asynchronously to send them to device
2. Heartbeat. The device requires a heartbeat within 30 seconds, but I can move this to library also
3. read_notifications. This handles processing push notifications in real time.
   Now that I think about it, I think all three of these can be moved to the library but their lifecycle can be managed by the integration so they start and stop correctly. I will update this.

And please don't forget, I am not here to call you out on stuff you did wrong, I am trying to point you at the relevant things that could increase the code quality and enhancing best practices. I am currently looking into how we can word best practices and present them as a guide to enhance the steps to create a new integration.

Thats fine actually I don't mind being corrected because I want to do it the right way. I do think that, with complex systems, there is a "right way" and I would love for HA to be more opinionated with that. HA should prescribe ways to do this with patterns rather than documenting functions. For example, in my case I am using a push method, so there should be one way to do this. Having documentation that points to existing code bases which are exemplars of this would be a good start. This is the feedback I see anyway, that people need to dive into core and just blindly copy existing integrations.

e.g in the getting started docs, have a section with common patterns:

Is your library pushing data to the integration? Here is how you set that up and here is an exemplar integration

Anyway, I pushed a change which moves more things into the coordinator and redid the unload stuff. Hopefully that is right. I am working on moving the queue stuff into the library

[joostlek]

I think all three of these can be moved to the library but their lifecycle can be managed by the integration so they start and stop correctly. I will update this.

Awesome!

I mean if you have a lot of passion for docs, feel free to contact me on discord so I have someone new ish who I can throw it to, to check if its understandable :P

Let me know when stuff is done, then we can take a look at it again :)

[iloveicedgreentea]

I think all three of these can be moved to the library but their lifecycle can be managed by the integration so they start and stop correctly. I will update this.

Awesome!

I mean if you have a lot of passion for docs, feel free to contact me on discord so I have someone new ish who I can throw it to, to check if its understandable :P

Let me know when stuff is done, then we can take a look at it again :)

Yes I would definitely like to help out. I would like to be involved in HA development more so this is a good first step. I can ping you on discord

I pushed some updates

1. Moved queues and tasks into library
2. corrected/updated type hints
3. removed name from config
4. fixed test failures
5. updated library to be more efficient and process updates better
6. added mypy to library to catch any silly stuff

I tested this locally and on my prod instance, everything works as expected (attributes, push updates, tasks, reload & disabling integration, etc)

Just a heads up once this is merged, I plan on opening the PR to add the binary_sensors and sensor platforms back in (those are also all working as expected but I can't add them because of the bot)

[iloveicedgreentea]

1. Removed extra attributes (will make PR to add sensors, binary sensors after this)
2. used common string references
3. removed duplicate unload logic
4. moved task and related logic to coordinator
5. addressed other misc comments
6. removed mac and WOL (intention is to set up device trigger in new PR)
7. changed library logic to use standby instead of power off (removes WOL dependency until its implemented)
8. simplified config flow