Make App.Call() wait for and return the result of the Home Assistant API call#47
Conversation
Some message types (e.g., `BaseMessage` and `AuthMessage`) used the full spelled-out word "Message" in their names, while others (like `ChanMsg` and `stateChangedMsg`) used "Msg". For consistency, rename the latter to be more consistent with the former: * `ChanMsg` → `ChanMessage` * `stateChangedMsg` → `stateChangedMessage`
Some users of this library will need to subscribe to messages and/or send and receive individual websocket messages. So make those interfaces public.
All messages have a "type" and "id", but not all messages have a "success" field. So remove the `Success` field from `BaseMessage`. Add a `BaseResultMessage`, which consists of a `BaseMessage` plus a `Success` field, to use in those cases when the message does have a "success" field.
Now that `BaseMessage` doesn't include a `Success` field, it is a building block that we can use within `stateChangedMessage`.
Since we're pretending that these messages have "success" fields, it is more appropriate to call them `ResultMessage`s. Soon, that assumption will be relaxed.
Not all messages coming from HA will be result messages, so don't try to parse them as such. Instead, only parse the `BaseMessage` part, and pass them to listeners as `Message` objects, which contain the `BaseMessage` part plus the entire raw message as JSON.
Now that nobody is using `ResultMessage`, we're free to make it more capable: * Add a new `ResultError` type, for holding errors that were included in result messages. * If a message contains an "error" field, parse it and store it in a new `BaseResultMessage.Error` field of type `ResultError`. * Change `ResultMessage` to have a `Result` field rather than a `Raw` field. Store the unparsed "result" field from a result message to `ResultMessage.Result`. * Add a method `Message.GetResult()`, which allows a `Message` to be unmarshaled as a `ResultMessage` and its result unmarshaled into a user-provided variable. If the message includes and error, return that error as a Go error. Soon this new functionality will be used to handle the results of HA API calls.
This agrees with `websocket.BaseMessage` and with the JSON field name.
Introduce a new type, `CallServiceMessage`, to contain the entire message required to invoke an HA service. This type embeds an instance of `BaseServiceRequest`. Remove the `ID` and `Type` fields from `BaseServiceRequest`, because they don't need to be set by the caller, but are rather managed within `App.Call()`. Move these fields to `BaseServiceRequest` by embedding a `websocket.BaseMessage` instance the the latter type.
Rename method `App.Call()` to `App.CallAndForget()`, because this is a "fire-and-forget" style of calling an API function that doesn't wait for a response. In a moment we'll add a new `Call()` method that waits for and returns the response from the server.
At the `Conn` level, we don't want to discard any messages entirely, because for all we know somebody might be interested in them. (In the future, there will indeed be "result" message listeners.) So instead, change `conn.Run()` to pass all messages through, but change the event listener callback to discard any messages that arrive that don't match the desired message type.
Add a new version of method `App.Call()` that not only invokes a Home Assistant API (like `CallAndForget()`), but also waits for the server to respond and unmarshals the result into a caller-provided variable. If the server returns an error, return that error as a `*websocket.ResultError`. This makes it easy to call APIs that return results, and also makes it easy for the caller to detect when the requested action failed.
|
Hey, sorry for the delay. I wanted to pull it down locally and try the refactor on my own automations to see how painful the breaking changes were and see what the API felt like. It was a bit painful but probably for the best to be able to detect errors. One request: I'd like to leave service data as a varadic arg. I know it could be classified as misleading, but I would argue most service calls don't need to use it, so peppering I also like keeping the type as |
|
We could add a log if |
OK, I'll make the change.
Technically, the requirement is that the type LightServiceData struct {
Brightness int `json:"brightness,omitzero"`
// ColorTemp is the color temperature in mireds
// (micro-reciprocal-degrees-Kelvin).
ColorTemp int `json:"color_temp,omitzero"`
}Then I can, for example, call This works because But if you prefer to leave this as |
|
Ah that makes sense, thanks for the example. Let's leave it as |
mhagger
force-pushed
the
call-returns-result
branch
from
046e4b3 to
177cc9e
Compare
Change the types of `serviceData` arguments from `...map[string]any`
to `...any`. This allows the caller to pass a struct in, as long as
the struct can be serialized to a JSON object. For example, for a
light, one might define
```
type LightServiceData struct {
Brightness int `json:"brightness,omitzero"`
// ColorTemp is the color temperature in mireds
// (micro-reciprocal-degrees-Kelvin).
ColorTemp int `json:"color_temp,omitzero"`
}
```
and then call `TurnOn()` like
```
_, err := app.Service.Light.TurnOn(
ctx, l.target, ServiceData{Brightness: newBrightness},
)
```
When calling a service, use `Call()` rather than `CallAndForget()` so that the result of the service call is collected. Return the result to the caller as type `any`. (If people go to the trouble of figuring out the structure of the methods' responses, these return types can be made more specific. This requires a `context.Context` argument to be added to the service call arguments. It can be used, for example, to limit the amount of time to wait for a response from the server.
mhagger
force-pushed
the
call-returns-result
branch
from
177cc9e to
28963af
Compare
|
I just force-pushed the branch, changing only the last two commits. The first of those now changes the type of The second commit is the same as before in spirit, though it had to be rewritten because of textual conflicts caused by the change to its parent commit. |
|
LGTM, thank you! |
2 participants
This PR starts by making the "message" types more consistent with each other and making them share overlapping parts of their structures.
Then it renames the existing
App.Call()method toCallAndForget(), and replaces it with a newCall()method that also waits for the server to return a result. (This involves temporarily subscribing to the message ID corresponding to the request.) Then it checks whether the result represents an error; if so, it returns a Goerrorto the caller that includes the error message from the server. Otherwise, it unmarshals the "result" part of the response into a caller-provided variable.This IMO is an important new feature, as it allows callers to ensure that the operation that they requested didn't fail, and also makes it easy to call APIs that return useful information.
I have retained
CallAndForget(), in case somebody really doesn't want to wait for the result of a call. Maybe there are some calls that don't return result messages? Maybe callers are worried about performance? (Though it should be possible to run multipleApp.Call()methods in parallel, which would seem to reduce that concern...)@saml-dev, part of this PR requires a change to the fix that you implemented in af77326 and 032922d (thanks for that, by the way!) which prevents "result" messages from being forwarded to event listeners. We can't discard those messages anymore, because the new
Call()method needs to receive them. So instead, letConn.Run()forward those messages to listeners, and instead change the event listener callback to drop messages that don't have the subscribed-to event type.I unfortunately have to repeat my usual caveat: I still can't run my own home setup on this branch, because it relies on other changes in #21 that haven't been re-rolled yet. So, while every commit compiles and example_live_test.go runs correctly at the tip of this branch, I can't do a lot more testing than that. I'm getting closer though, and hope soon to migrate my own home setup to a branch based on your
main, which would allow me to do more real-life testing. I really appreciate your patience and the bug fixes that you have applied on top of my earlier PRs and hope that I'm not trying your patience!