dahlb / ha_hatch

Home Assistant Integration for Hatch Rest Mini
MIT License
76 stars 16 forks source link

fix: One media entity for all; minor refactorings; support RestIot in rest_media_entity.py along with favorites + specific sounds from app #121

Closed ChrisCarini closed 1 week ago

ChrisCarini commented 1 month ago

Works towards fixing https://github.com/dahlb/ha_hatch/issues/95 (the necessary custom_component changes)

Tylerbb812 commented 1 month ago

You should request review so this can get pulled in :) waiting patiently haha

ChrisCarini commented 1 month ago

You should request review so this can get pulled in :) waiting patiently haha

It won't let me mark a reviewer, though I would love it if @dahlb could take a look at this and provide some thoughts :)

dahlb commented 1 month ago

@ChrisCarini I was waiting to review this until the API changes review is finished. I generally prefer a bottom up design but could provide a partial review while we wait for that if you'd like.

ChrisCarini commented 1 month ago

@ChrisCarini I was waiting to review this until the API changes review is finished. I generally prefer a bottom up design but could provide a partial review while we wait for that if you'd like.

No strong preference - if there are obvious things you see that don't depend on the other PR, happy to start fielding those now in pursuit of being as async as possible, but if you'd prefer to wait, I'm ok with that, too. I'm responding to your most recent comments on the other PR, sorry for the delay.

dahlb commented 3 weeks ago

@ChrisCarini any updates on this? there seems to be a lot of interest in getting this merged soon.

ChrisCarini commented 3 weeks ago

@ChrisCarini any updates on this? there seems to be a lot of interest in getting this merged soon.

Hi @dahlb - no updates yet, I've seen the recent increase in activity on the respective issue, though.

Essentially, I just haven't had much time (or access, really) to test any changes myself - my preference would be able to make any changes and self-test before making it available for others to test (it's faster to iterate on changes that way, less overhead). If we were to flip that, I could make the changes I think might fix the issues people are seeing (and some issues I'm seeing myself), but would want others to be able to make those changes as well to test themselves.

As mentioned before, the free times I have available for development here strictly overlap with the times the sound machine is in active use :( It'd be really nice if I could just borrow a second hatch for a few weeks, but I know that's not very realistic 😅

What do you think about my suggestion for others to self-change above (I'm personally a bit hesitant because it seemed folks needed the package to be uploaded for easier install into their HA setup)?

dahlb commented 3 weeks ago

From what I've read which isn't everything in the thread it seems the existing gen2 support in the integration doesn't work, it is commonly the problem that users have difficulty testing from branches.

Since the integration doesn't work anyway for gen2 I'm open to releasing what you think is working and letting users provide feedback.

This PR should be renamed from fix to "feat: added favorites and sounds from app for gen2 rest+" to more easily let people track a large change from a versioning stand point.

ChrisCarini commented 3 weeks ago

I'd generally be open to the idea to release and let folks test, however I've personally had major issues recently. Specifically, (a) my scripts using these entities randomly hanging on various steps involving these entities (doesn't seem to always be the same entity), and (b) the actions generally not working (they will show as fine in the timeline view, but the actual expected chance does not happen). It's because of these two that brings me pause.

Let me see if I can find some time this week to make some progress - I believe there are a few outstanding suggestions I'd like to incorporate already - and we can assess from there. How does that sound?

dahlb commented 3 weeks ago

That sounds good. I can help with any of the refactoring stuff if you want. I've settled back in since finishing my trip to Japan and have about 3 hours a week. I don't have a gen2 to test with but could do any refactoring related stuff pretty quickly.

ChrisCarini commented 1 week ago

Thanks for your work towards getting this fixed in https://github.com/dahlb/ha_hatch/compare/v1.17.1...v1.19.2 :) If i squint hard enough, I see some of the changes may have been inspired from this PR... maybe :)

Anyways, thanks for the review & discussions!