search

npope / home-assistant-crestron-component

Integration for Home Assistant for the Crestron XSIG symbol
Apache License 2.0
59 stars 20 forks source link

Minor code change for 2 problems I rand into when integrating #9

Open natethelen opened 1 year ago

natethelen commented 1 year ago
  1. Fixes a problem when a value_template in a digital to_joins was resulting in update_result being an actual boolean value and consequently it was never getting sent to the Crestron system: value_template: "{{is_state('lock.my_lock', 'locked')}}"
  2. Fixes a problem during startup where result was a None value and consequently an error was showing up in the logs when trying to turn it into an int.
natethelen commented 1 year ago

Added another change:

  1. Fixes a problem that happens when an unknown source comes in from Crestron. An exception is generated which breaks the analog callbacks resulting in no further analog updates registering on the HA side. The media_player should respond to an unknown source by setting source to None. Unknown sources are a valid possibility as users may not want HA and its users to be able to select sources like doorbells and intercoms.
adamjs83 commented 1 year ago

Added another change:

  1. Fixes a problem that happens when an unknown source comes in from Crestron. An exception is generated which breaks the analog callbacks resulting in no further analog updates registering on the HA side. The media_player should respond to an unknown source by setting source to None. Unknown sources are a valid possibility as users may not want HA and its users to be able to select sources like doorbells and intercoms.

Will this solve a problem I have of the integration basically crashing and needing to reboot homeassitant whenever an unexpected value comes in?

natethelen commented 1 year ago

Added another change:

  1. Fixes a problem that happens when an unknown source comes in from Crestron. An exception is generated which breaks the analog callbacks resulting in no further analog updates registering on the HA side. The media_player should respond to an unknown source by setting source to None. Unknown sources are a valid possibility as users may not want HA and its users to be able to select sources like doorbells and intercoms.

Will this solve a problem I have of the integration basically crashing and needing to reboot homeassitant whenever an unexpected value comes in?

Probably not. This addresses a specific case where an unexpect value coming in as the source of a media_player and keeps the analog value crash from happening. Work needs to be done to better handle exceptions so that the callbacks functionality doesn't short circuit. I will take a look at it soon and see if I can figure out a good fix

adamjs83 commented 1 year ago

Awesome. I currently have my Crestron component in a dedicated HA VM throwing the entities into my primary HA through Remote Home Assistant. Whenever one of them goes unavailable for more than 30 seconds an automation reboots the dedicated HA which brings everything right back up. Its a little clumsy but it works. Let me know if you do end up working on it if any of my logs would help. I'm not a programming expert by any means but when I had asked about this on the HA development forum someone had recommended something about properly developing it to use config flow and then being able to reload the integration directly without rebooting HA.

natethelen commented 1 year ago

Yes, I noticed that, too (about it not being in the format that current ones are). I wonder if @npope would mind if I took much of his code and made a new extension...

npope commented 1 year ago

@natethelen I was actually just starting to work on updating the component to be config-flow based so that (1) it can be reloaded without rebooting HA and (2) we can add things like device triggers. Happy to collaborate on that here via PRs or whatever works for you.

adamjs83 commented 1 year ago

@natethelen I was actually just starting to work on updating the component to be config-flow based so that (1) it can be reloaded without rebooting HA and (2) we can add things like device triggers. Happy to collaborate on that here via PRs or whatever works for you.

As someone who can add no assistance in updating the code but relies on it everyday I appreciate the effort you guys put in and making this freely available.

natethelen commented 1 year ago

@natethelen I was actually just starting to work on updating the component to be config-flow based so that (1) it can be reloaded without rebooting HA and (2) we can add things like device triggers. Happy to collaborate on that here via PRs or whatever works for you.

@npope I would love to help out with that. I am an experienced professional Python engineer with a bunch of personal experience in home automation. Just now getting my hands wet with Home Assistant, but very happy with it so far. DM me if you want to move forward. I can devote some side hours to the project.