RFP: Clarification on Generic Subcommand Index Handling

[qdot]

We don't currently have any rules about how we handle repeated subindexes in Generic Commands.

For instance, with VibrateCmd, say someone creates the following packet:

{
"VibrateCmd":
{
"Speeds": [
{ "Index": 0, "Speed": 0.5 },
{ "Index": 0, "Speed": 0.75 }
]
},
"Id": 10,
"DeviceIndex": 0
}

Our implementations in both C# and JS would currently send 2 commands to the device back to back, which is silly. We should specify whether either the last value for each index should be chosen, or whether we should error out on this case.

I don't have strong feelings either direction. Erroring out would require actual library logic though, I don't believe we'd easily be able to catch that via the schema.

[blackspherefollower]

It should probably be disallowed, implemented at the message handling layer within the drivers (since that's were we disallow too many and out of bounds subcommands).

Given that this logic is repeated so often, maybe a generic utility function should be written to handle these rules?

[qdot]

Mostly looking for spec clarification here, library specifics can be handled in the relevant repos.

My comment about the schema was mostly about the ease of the possible fix for this, because until we get a reference set of messages that we can use to test libraries (buttplugio/buttplug-schema#2), it's hard to make sure everything works and is at parity.

[blackspherefollower]

Ah, got it. Then in my opinion, we should document that only one subcommand per feature should be sent.

[hiss-remi]

I can't even imagine a scenario where someone would want to send a message like you're opening example. Just what could it be intended to accomplish? I think it should be an error too, since most times it would probably be a silent bug otherwise.

[qdot]

Yup, only reason I was RFP'ing this in the first place is that we already shipped the message in January and I wanted to make sure I wasn't missing anything. I think we can refine the spec without rolling the version on this, since the logic will be required in the server instead of the schema anyways.

I'll update the spec tomorrow and land then, will close this with the commit message for it.

[qdot]

This should be checked as part of is_valid() for all generic device messages. Filing for v6.1