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

ProcessLink() should probably get passed framework.Client #1

Open
anthonyrowe opened this issue Dec 31, 2017 · 1 comment
Open

ProcessLink() should probably get passed framework.Client #1

anthonyrowe opened this issue Dec 31, 2017 · 1 comment

Comments

@anthonyrowe
Copy link

Seems like it would be pretty common when linking devices to want to access thinks like the device name and owner string. Passing framework.Client structure, or some reduced clone of it, would make that easier. UnLink probably doesn't matter as much, but might as well make them symmetric.

I'm not sure if they should also be accessible in ProcessMessage(), maybe? The downside would be if people abuse the Client calls it will slow things down. We don't want a REST call for every MQTT message. A warning in the comments might be enough though.

@linux4life798
Copy link
Member

linux4life798 commented Dec 31, 2017

I agree. Not providing an easy way to access the ServiceClient object was a bit of an oversight.
Right, we don't want people to call that FetchDeviceInfo frequently, so I think we should facilitate fetching cached info or something similar.
I needed to make that FetchDeviceInfo seperate and explicit, since it currently DOES require a separate REST call to get that basic device info. Furthermore, it is not guaranteed to succeed after a service link add event.

I propose adding more essential device info to the service link event sent from the REST server. Then, I could add that info to the ProcessLink hander without feeling guilty about adding racey code to a core library. I think we should provide those basic properties to all the handlers. This could definitely simply user code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants