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

update SystemManager.listen and listenDevice logic #953

Closed
wants to merge 16 commits into from

Conversation

raman325
Copy link
Contributor

@raman325 raman325 commented Jul 14, 2023

is it as simple as making this change (calling listen and listenDevice from PluginAPI)?

  • Also added stubs for PluginAPI and related interfaces
  • Fixed some types

@koush
Copy link
Owner

koush commented Jul 14, 2023

EventListenerRegister is currently code gen as:

class EventListenerRegister:
    """Returned when an event listener is attached to an EventEmitter. Call removeListener to unregister from events."""

    def removeListener(self) -> None:
        pass

EventListenerRegister as returned from the api object is actually async since its an rpc call.

class EventListenerRegister:
    """Returned when an event listener is attached to an EventEmitter. Call removeListener to unregister from events."""

    async def removeListener(self) -> None:
        pass

All calls to the rpc api object are async. So you'll need to shim it on return to prove a non-async version from the api that calls run_coroutine_threasafe on the plugin event loop. That's what typescript does.

@koush
Copy link
Owner

koush commented Jul 14, 2023

You'll also need to do similar for the SystemManager.listen and listenDevice calls (which return the event listener registers and are also async):

class SystemManager:
    """SystemManager is used by scripts to query device state and access devices."""

    async def getComponent(self, id: str) -> Any:
        pass

    def getDeviceById(self, id: str) -> ScryptedDevice:
        pass

    def getDeviceByName(self, name: str) -> ScryptedDevice:
        pass

    def getDeviceState(self, id: str) -> Any:
        pass

    def getSystemState(self) -> Any:
        pass

    def listen(self, callback: EventListener) -> EventListenerRegister:
        pass

    def listenDevice(self, id: str, event: str | EventListenerOptions, callback: EventListener) -> EventListenerRegister:
        pass

    async def removeDevice(self, id: str) -> None:
        pass

@koush
Copy link
Owner

koush commented Jul 14, 2023

Tbh the eventing being a faux synchronous call is not great, I think I'd also prefer this was an async iterator pattern too (which I only recently added support for in the rpc protocol). Garbage collection, throwing/returning inside an event iterator, and plugin death would trigger the unsubscription automatically.

But I'll deal w/ that later probably. The async issue will need to be resolved. Or not, you can simply just await it (or it won't run otherwise) since you are the only consumer of the python api right now and know that its a coroutine.

@raman325
Copy link
Contributor Author

agreed this should switch to async. It's probably ok as is in isolation but HA wants to manage the event loop and I think faux synchronous will end up triggering some errors although I haven't tested that yet

@koush
Copy link
Owner

koush commented Aug 9, 2023

need to undo all the typing related changes as it will not work on older versions of python. I had to revert some off our prior commits to fix crashes.
that's all internal API stuff anyways and doesn't need typing from the sdk consumer.

there's also a new circular reference between types and other which may or may not cause issues, unsure.

@raman325
Copy link
Contributor Author

raman325 commented Aug 9, 2023

yeah I'm moving this PR to draft, it's been a while since I've been able to work on it and I may have to start over, but I don't want to lose sight of it

@raman325 raman325 marked this pull request as draft August 9, 2023 04:47
@raman325
Copy link
Contributor Author

raman325 commented Aug 9, 2023

there's also a new circular reference between types and other which may or may not cause issues, unsure.

this won't matter, it only gets triggered when type checks run which don't suffer from the circular reference problem. Your point about older versions of python is fair though

@raman325 raman325 closed this Jul 24, 2024
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.

None yet

2 participants