i8beef / node-red-contrib-castv2

MIT License
22 stars 14 forks source link

[Feature Request] 2 cases of spurious (potentially unwanted outputs) #98

Open illuzn opened 6 months ago

illuzn commented 6 months ago

Please add 2 configuration checkboxes to the node as follows:

Name Default Description
Status Report on Connect true This will provide the status report msg that occurs on first connecting to a device. This msg can cause unwanted triggering of automations (e.g. if device loses then regains connection).
Subscribe to Status Reports true This will provide the status report msg that occur after connection upon a change occuring e.g. new media playing. Again, these status reports can cause unwanted triggering of automations.

I'd give these a crack myself but my javascript is basically non-existent.

Example flow for context:

[{"id":"7e5bfb8fa4c126e8","type":"castv2-sender","z":"ee650b50714e44e9","name":"","connection":"","x":500,"y":200,"wires":[["9321ebf9139067ac"]]},{"id":"51104fa27227dd38","type":"castv2-sender","z":"ee650b50714e44e9","name":"","connection":"","x":500,"y":260,"wires":[[]]},{"id":"9e5878b11fc8abbd","type":"inject","z":"ee650b50714e44e9","name":"","props":[{"p":"payload"}],"repeat":"","crontab":"","once":false,"onceDelay":0.1,"topic":"","payload":"{\"type\":\"VOLUME\",\"volume\":\"50\"}","payloadType":"json","x":270,"y":260,"wires":[["51104fa27227dd38"]]},{"id":"9321ebf9139067ac","type":"debug","z":"ee650b50714e44e9","name":"debug 2","active":true,"tosidebar":true,"console":true,"tostatus":false,"complete":"platform","targetType":"msg","statusVal":"","statusType":"auto","x":700,"y":200,"wires":[]}]

Reproduction steps:

  1. Complete the connection details for a new chromecast device. Deploy.
  2. Debug output will show a msg generated on connection.
  3. Press inject.
  4. Debug output will show 4 (or 2 or 1) generated msg. The first is desired and is the status report assume the volume change was successful. Now is when the behaviour diverges. My original google home mini generates only 1 output message - presumably desired. My "works with google" bose speaker generates 2 output messages (I assume this is because you are setting the volume to a fixed 50% but the steps are actually different from this so the second message is the device falling in line with the steps - this is only speculation though). My google nest hub max generates 4 output message (no idea why).

In any event these extra messages are undesirable, and I'd suggest the behaviour should be to output msg.platform with SUCCESS/FAIL depending on the result.

i8beef commented 6 months ago

I think I can accommodate these requests, I just have to think about them for a second for implications. Its good to remember that the cast protocol is more an async eventing API than a request/response protocol. While I do everything with a single node to keep things simple, there's an implied separation of what you sent INTO the node vs what it sends OUT. While requests may LOOK like they get a response that I output, its just the device BROADCASTING a "status change" that occurred because of your command, ie, there's some nuance here, and I intend to expose the cast protocol more or less unmolested rather than building a wrapper / state management on top of it.

Put another way, I tend to expose EVERYTHING so that YOU have choices in how to handle them instead of me handling them silently.

But your requests seem reasonable and might work within that framework... I'll take a look at this soon (maybe this weekend).

illuzn commented 6 months ago

While requests may LOOK like they get a response that I output, its just the device BROADCASTING a "status change" that occurred because of your command, ie, there's some nuance here, and I intend to expose the cast protocol more or less unmolested rather than building a wrapper / state management on top of it.

Just a note, this is obviously untrue for GET_VOLUME and GET_CAST_STATUS where msg.platform is passed back together with my original msg.payload. Hope I'm not overstepping here - just a little hint when you are considering how the framework should be.

Edit: Also, any thoughts/ ideas on why different devices are producing 1/2/4 output messages for a volume change?

i8beef commented 6 months ago

Possible I'm misremembering the reason then. It's been a while since I've been in here.

i8beef commented 6 months ago

Ok took a minute to look. Yes, there's some nuance under the hood due to the underlying cast library imposing a request/response model on top of the Cast protocol, which I then unwrap and present back in an eventing model in the node itself (options were limited when choosing a functional cast library).

You should NOT think of that as request / response though, you should think of those three "solicited" cases (GET_STATUS, GET_CAST_STATUS, and GET_VOLUME) as just asking the cast device to publish its current status again when it gets a chance. Yes, those three are implemented under the hood as request/response, but its easier to think of it THIS way because EVERYTHING ELSE works that way too, and pains have been taken to make the contract here consistent.

The proper model to think of here is:

  1. Any status update event the cast device publishes, I publish
  2. You can SOLICIT a status event to be published on demand
  3. Any command you send CAN cause a status change, which triggers the cast device to publish one OR MANY status change event(s)
  4. On any connection, the current status will always be published to maintain the contract that "the last message published is reliably the current status"

You can't really disable unsolicited messages without compromising this model. The ONLY thing it would publish then are the solicited responses (ie, you could still run a command, but nothing would get emitted except for those three exceptions EVER). Can you explain how that would help your case? Maybe I can help you work WITHIN this model instead of needing changes?

==

As for why certain devices emit more messages than others, its because the cast protocol doesn't prescribe a standard and different devices do different things. Some publish extra events because the cast protocol model means that YOU are expected to just be able to deal with any number of status updates at any time, so they do whatever they want with it.