Conversation
The old code always created URIs with the following patterns:
* `http[s]://{ip}:{port}/api/[…]`
* `ws[s]://{ip}:{port}/api/websocket`
where `{ip}` and `{port}` are externally-specified parameters. But if,
for example, the code is running within a supervised container, then
it is supposed to connect via the proxy at
* `http://supervisor/core/api/[…]`
* `ws://supervisor/core/websocket` or `ws://supervisor/core/api/websocket`
It is not possible to generate such URIs with the existing
code (because of the extra `core/` in the path).
So instead, continue a little bit further with the idea of
saml-dev#17: add a second way
to initialize the app, where the caller can specify the URIs itself.
This is implemented via a new constructor function
`NewAppFromConfig()`, which takes a new configuration type,
`NewAppConfig`, as argument. Rewrite `NewApp()` to call
`NewAppFromConfig()`.
Remove the `ctx` arguments from the following functions. The arguments are not used, and there doesn't seem to be much prospect for making these functions cancelable: * `websocket.WriteMessage()` * `websocket.ReadMessage()` * `websocket.SendAuthMessage()` * `websocket.VerifyAuthResponse()` * `websocket.SubscribeToStateChangedEvents()` * `websocket.SubscribeToEventType()` * `websocket.ListenWebsocket()`
Instead of creating and returning a context, change these functions to take a context as argument.
Instead of creating a context in `NewAppFromConfig()` and `NewApp()`, accept a context as argument and use it only for connecting. (It was never used for anything else anyway.)
Make these two functions methods of `App`.
Introduce a new interface, `scheduledAction`, and make `*DailySchedule` and `*Interval` both implement it. Change `App` to have a single queue of `scheduledAction`s.
Let's make it the only interface to the `websocket.Conn`.
In particular, don't make the caller pass in a `websocket.Conn`.
Instead of using public functions, make these into private methods.
Make the subscribe functions into methods of `websocket.Conn`.
Extract type `JWaveJSEventData` from `EventZWaveJSValueNotification`.
The entity state should always be a string, so add a string-based helper class to hold it.
Change `CompressedStateChangedMessage.Apply()` from a method into a generic top-level function, which can take an arbitrary `Entity` type as argument and apply the change to it. This involves some type conversion, which is done by serializing to and from JSON a couple of times, but at least the application code doesn't need to be written separately for each type.
|
Hi folks, I assume none of the commits have been merged and now there are newer conflicting commits pushed. @mhagger do you have an own fork that can be used and has your changes? It'd be nice if we could merge the two. |
|
Hey I'm sorry I never responded! This came in right after my son was born and I forgot about it 🤪 I'm happy to review and merge smaller PRs, I'll just never have time to review a PR with 58 commits |
|
When I didn't get any response to this PR, I continued work on my own copy of this branch but didn't push the new commits to this PR. Since there seems to be a little bit of interest (or morbid curiosity?) now, I just pushed about 20 more commits. That is the version that I have been running in my own home for about 15 months, mostly without problems.
@endreszabo: My changes are all in my own fork on branch
@saml-dev: I certainly don't expect anybody to merge a PR like this with so many commits (looks like 103 of them now!). I originally created this PR to start a conversation about whether you want changes like this at all, and if so how important backwards compatibility is, what direction you want the project to go, etc. Let talk about those meta-topics, and then based on those answers we can figure out if it makes sense for me to break up this patch series into small pieces that can be rebased to the current I suggest that you read the original PR comment and maybe glance over the commit messages to get an overall idea of whether such changes would be interesting to you at all, and maybe give me some guidance on if/what changes you'd like submitted as smaller PRs. I can't promise when I can get back to it, but I do enjoy this project so I'd at least try to find the time. But if you're not interested, I won't bother. My fork is public in any case so people can use it from there if they like (branch "whirlwind"). What I'm doing with gome-assistantIn case you're curious, I've used this package, with my modifications, for two things, mainly:
I know that this isn't a unique sort of application, but I really think that the Go programming model is great for handling this kind of thing. I've got a goroutine corresponding to every button and light in my house! To get this working, it was helpful to add easy two-way communication between HA and gome-assistant. I haven't published that other code, but if there's interest I could maybe clean out the configuration specific to my house and publish it as an example, along with documentation for building it and running it as an Add-on. |
Congratulations on your son! No worries, family comes first! |
|
Thanks for the quick responses folks :)
My man :)
Yeah, I've seen it and that you put a lot of effort in revamping and adding additional functionality and that's why I pursued to have the branches merged to some extent. I currently have a huge (7k+ SLOC TypeScript) automation framework/script running 'in prod' that I wish to port to Go as I'm getting more familiar with Go. It has many features we all could benefit from. But I would like to build upon strong/established/proven fundamentals :)
I created a TS library called TheButton to cover this very same problem. It has now evolved into other kind of control surfaces like Stream Deck. I can also do soft-repeating for remotes that does not support any kind of repeating. Or does repeating wrong, like the Hue remotes.
I do the same, even the minute interval is the same. The curve is calculated using Spline and some keyframes throughout the day: cubicSplineColorTempIntervalEmitter.setValues([
0, //midnight
sunTimes.dawn.valueOf() % 86400000 - 3600000,
sunTimes.dawn.valueOf() % 86400000,
sunTimes.sunriseEnd.valueOf() % 86400000,
sunTimes.sunriseEnd.valueOf() % 86400000 + 3600000,
86400000 / 2,
sunTimes.sunset.valueOf() % 86400000,
sunTimes.dusk.valueOf() % 86400000,
sunTimes.night.valueOf() % 86400000,
86400000 // next midnight
], [
nightTimeMireds,
nightTimeMireds,
300,
dayTimeMireds,
dayTimeMireds,
dayTimeMireds,
300,
320,
nightTimeMireds,
nightTimeMireds
])It is just awesome how natural it feels.
I did the above three using the concept of prioritized light filters/layers which I am very proud of. It is a bunch of callbacks or series-of-plugins if you will that renders the state of the lights.
Indeed, that's my goal to have goroutines handle such events.
I would be really interested to see your code. |
@endreszabo: It took some tidying up, but I just published that code that I wrote that uses my fork of gome-assistant as mhagger/hassioneer. I hope that it's helpful to somebody. I don't plan to put a lot of work into it beyond what's needed for my own home, but maybe the code and the instructions for building it and getting it running as a Home Assistant add-on will help people get over that hump with less friction! Let me know if you have any feedback. |
@endreszabo thank you! 😁 Onto @mhagger's contributions. I am definitely open to any changes that bring the library more in-line with idiomatic Go. gome-assistant was the first Go code I ever wrote, I used this project as a way to learn the language. I'm more familiar now but haven't taken the time to go back and update it and likely won't, so contributions like that are certainly welcome! I'd also welcome fixes like number 9 that solve the incrementing ID issue. I used to run into this and remember trying to fix it but honestly don't remember if I did, not sure I ever verified it. New library features can be added as well, I'd like to stick to the spirit of the library and keep breaking changes to a minimum if possible, but I never tagged a 1.0 release so it's ok if there are some that add a lot of value. Thanks for contributing! |
|
@saml-dev: thanks for your response. Then I propose (as time allows) to start rebasing my changes onto the current If so, maybe we can leave this PR open as an aid to keeping track of overall progress, but switch it back to "Draft" state as it's currently not close to mergeable. |
|
Sounds great to me! |
|
And agreed on "as time allows", that's all I can commit to as well so no rush at all. |
|
I've put this PR back in draft mode, since I'm breaking it up into smaller pieces to make them easier to digest. I'll leave this one open for now because I think that it's convenient for keeping track of the big picture, but feel free to close it if you prefer. |
3 participants
Hi! Thanks for this project! I've been working towards using it for my own home automations. I made a lot of changes to this library along the way that might be interesting to you. I don't really expect you to merge this whole pull request, because I have made a lot of changes in many different areas and didn't always retain backwards compatibility (though I haven't changed the interface very dramatically). Also, while the commit series is fairly logical, there's a little bit of back-and-forth as I work a couple of the tricky points out.
I'm submitting this as a PR anyway, in case you would like parts of the series (e.g., I could pull apart smaller PRs for you), or just want to get some ideas, or whatever. I hope that it's useful, but if not, don't feel guilty about declining it.
I'm using this library within an add-on that I am writing for my own use (not yet published). I'm running it in a supervised container on my Home Assistant Yellow, which itself required a couple of changes to the package. I've been happy with the setup so far, at least within the constraints of what add-ons can do. I totally like the idea of using Go to write automations, since the threading model seems like such a good fit for the message-driven architecture. At some point I will try to document how I build and run the Go-language add-on, since that took quite a lot of fiddling to figure out.
The commits are each mostly small and self-contained and somewhat documented. I recommend browsing them commit by commit. Here I'll summarize some of the big themes, in rough commit order:
ctxargument should always be a function/method's first argument. Make that change.ctxargument from some places where it isn't used.ctxarguments to the functions that set up a websocket connection.WebsocketWritertowebsocket.Connand make it more capable.websocketConnto manage its own subscribers.Messagetypes, all embeddingBaseMessage, which implements its ID to be changed through aMessageinterface.Apptype. These manage message IDs, subscribing, and unsubscribing internally to connect requests with responses and make the calls synchronous from the point of view of the caller.Call(): send an arbitrary message and wait for and return a single response.CallService(): send acall_servicerequest, then wait for and return a single response.Subscribe(): subscribe to some kind of notifications, and return the immediate response, but leave a subscriber subscribed to subsequent messages.Appto a newapppackage, to make space at the top-level package for more general things that can be imported by any subpackages without introducing circular dependencies.Latitude()andLongitude().Stateinterface in more places.App.ServiceandApp.Statepublic. This means that callbacks only need an*Appargument rather than separate service and state args.Step 9 deserves more explanation. Home Assistent insists not only that message IDs be distinct, but also that they be strictly increasing. This means that, in a multithreaded program, you can't do this sort of thing:
The problem is that a different thread might jump in just after you allocate your ID, and allocate its own ID and send its own message, and then by the time you get around to sending your own message, your ID might not be usable anymore. I actually ran into this bug, in a less trivial situation.
My solution to that problem (after one or two false starts) is to add a
conn.Send()method, which invokes a callback function while holding a lock. It passes that function awebsocket.LockedConn, which can provide it with IDs and can send messages for it. The callback can thus send messages without worrying that another ID will be allocated by another thread. This is reminiscent of Go standard library'ssyscall.RawConnmechanism.Happily, all of this is wrapped in such a way in step 12 that most callers (e.g., of
App.Call(),App.CallService(), andApp.Subscribe()) don't have to worry about those details, but at the same time multiple calls can be in progress at the same time. I find the result very intuitive to use. IMO this is the nicest improvement in the series: now you can invoke a service and get a response, which opens up a lot of possibilities, like asking Home Assistant for its list of entities, areas, devices, etc. and acting on the result. I haven't added wrappers for such service calls yet but it's easy to do.Anyway, let me know what you think and whether I can help with anything.