buttplugio / buttplug

Rust Implementation of the Buttplug Sex Toy Control Protocol
https://buttplug.io
Other
873 stars 61 forks source link

Buttplug Protocol Spec v4 Update #565

Open qdot opened 1 year ago

qdot commented 1 year ago

Buttplug v4 is mainly a refinement of device enumeration from the server to the client, in order to make device information clearer to client developers, and allows them to write simpler and more expressive APIs on top of the protocol.

TODO List:

qdot commented 5 months ago

Ok, I'll consider the config v2 -> v3 work part of this bug, so can put some discussion here. I don't want to fill up the config discussion with project formatting issues.

@blackspherefollower If you're curious what the new config looks like, check the config-rewrite-minimal branch. I've made separate directories for v2 and v3 config for now, we'll remove v2 once v3 is solidified.

Inheritance still exists in the config, but at the "top" level. Name, description, and features can be overridden, but this time it's an all or nothing thing. v2 allowed overriding per-message, where now if any features change in a specific configuration, all features (even those in the default) must be added to the configuration. The inheritance system has also been completely removed from the configuration code which makes life WAY easier (and all devices are expanded to their full definitions when we load the file). I think simplify things over all.

I wrote a script to convert the v2 yaml to v3, basically filling in all of the missing inheritance values. This seems to have worked, inasmuch as protocol tests pass. That said, it did blow away all of the comments in the v2 file when moving to v3. How much of those do we want to keep around?

qdot commented 3 weeks ago

Proposed changelog of the v4 spec:

https://spec-v4--docs-buttplug-io.netlify.app/docs/spec/changelog

qdot commented 3 weeks ago

Problems the current v4 spec proposal doesn't solve:

blackspherefollower commented 3 weeks ago

For the Solace* issue, I'd still like to suggest we consider OscillateWithRange as sketched out in https://github.com/buttplugio/buttplug/issues/611

This would actually be applicable to both Solaces, the Keon, the Hismith Servo, the Fredorch and possibly a few more linear devices natively (as in they have built-in support for this).

Other linear devices "could" have simulated support, but I've had issues with this client side due to bad timing so I'm not going to push for that.

qdot commented 3 weeks ago

@blackspherefollower Ok, so the one problem I have with oscillate with range:

What developer do you think will actually use that message?

blackspherefollower commented 3 weeks ago

Hmm... Outside of the Solace and servo (which don't support linear positional movement), I guess most clients would either just set full range or use LinearCmd.

The loudest use case has been OGB, but if the Solace dies off, the client control aspect likely goes too.

I think users wanting to set their own range limits is probably still a valid ask, but that becomes just a setting that doesn't need to be part of the protocol.

qdot commented 3 weeks ago

@blackspherefollower So the one interesting thing I can think of for it: Something like supporting strokers in general the GHR. Being able to set speed and stroke range means the ability to... sort of edge with a stroker? 'cause you can keep speed the same and adjust stroke range to be shorter/longer as an intensity change while still keeping the user hard.

Another problem with OscillateWithRange is that it also introduces a message with 2 bounded step ranges. This is really just poking at the issue we have with not being able to vary ScalarCmd types within an actuator. It feels like this could be doable with two ScalarCmd commands, one for speed, one for range, but we'd need to express that as 2 separate actuators, which gets weird when we're trying to relay that info to a client to case up in a nice API.

blackspherefollower commented 3 weeks ago

That's an interesting use case, but I think it could be implemented today with LinearCmd.

I agree that having even an optional extra parameter to ScalarCmd doesn't make sense, but the rename to StaticCmd does also add more flexibility (it no longer implies a single parameter and with well defined actuator types, the shapes "could" differ)

qdot commented 2 weeks ago

That's an interesting use case, but I think it could be implemented today with LinearCmd.

For anything that already takes linear, sure. For the Solace and the machines, not so much. This would bridge the two.

That said: What about multiple actuators/sensors, differentiated by type, per feature?

As usual, the problem here is "how to present this to developers in a way that anyone will give a shit and actually use it," but at this point it also gives client developers the option to either expose or ignore without having to really consider implementing other messages.

blackspherefollower commented 2 weeks ago

So, you've already convinced me that although something like "oscillate-with-range" would be useful for the Solace and the Servo, no integration is likely to use it.

I do think that a mod dev might want to use oscillate on a device that also supports linearcmd: most likely they'll just go for the full length, so the range is likely irrelevant or they'll implement their own oscillation with settings to tune the timings (my own experience with trying to do this with the handy was that the handy was slower than duration provided, and I bet that most linear devices will have a certain amount of error in terms of how far they lag under load).

So having features per actuator does feel like a clear way to say "use either of these commands for this actuator - if you use both, you'll clobber the last command"

Yoooi0 commented 2 weeks ago

I tried to also come up with something reasonable that works with v4 but tbh everything feels clunky. Not sure if its possible to add configurable properties to v4 in a clean way without another redesign.

I think Best solution I got is to have multiple actuator+sensor pairs per feature:

{
    "DeviceName": "Sample Device",
    "DeviceFeatures": [
        {
            "FeatureName": "Oscillate",
            ???: [
                {
                    "Actuator": {
                        "ActuatorType": "Oscillate",
                        "ActuatorName": "Oscillate Speed",
                        "ValueRange": [0, 1],
                        "MessageTypes": [ "ScalarCmd" ]
                    },
                    "Sensor": {}
                },
                {
                    "Actuator": {
                        "ActuatorType": "Range",
                        "ActuatorName": "Oscillate Range",
                        "ValueRange": [0, 100],
                        "MessageTypes": [ "RangeCmd" ]
                    },
                    "Sensor": {}
                }
            ]
        }
    ]
}

But that still does not feel right, oscillate speed/range for me is a property of an actuator, but not an actuator itself.

Or what about this:

{
    "DeviceName": "Solace",
    "DeviceFeatures": [
        {
            "Actuator": {
                "ActuatorName": "Oscillate",
                "ActuatorProperties": [
                    {
                        "PropertyName": "Oscillate Speed",
                        "ValueRange": [0, 1],
                        "MessageTypes": [ "StaticCmd", "LinearCmd" ]
                    },
                    {
                        "PropertyName": "Oscillate Range"
                        "ValueRange": [0, 100],
                        "MessageTypes": [ "RangeCmd" ]
                    }
                ],
            }
            "Sensor": {
                "SensorName": "Oscillate Position"
                "ValueRange": [0, 100],
                "MessageTypes": [ "ReadCmd", "SubscribeCmd" ]
            }
        },
        {
            "Sensor": {
                "SensorName": "Battery",
                "ValueRange": [0, 100],
                "MessageTypes": [ "ReadCmd", "SubscribeCmd" ]
            }
        }
    ]
}
qdot commented 2 weeks ago

Here's the cursed structure I was rotating in my mind:

{
    "DeviceName": "Solace",
    "DeviceFeatures": [
        {
            "Actuator": {
                "Oscillate":  {
                    "PropertyName": "Oscillate Speed",
                    "StepRange": [0, 20],
                    "MessageTypes": [ "StaticCmd" ]
                },
                "Range": {
                    "PropertyName": "Oscillate Range",
                    "StepRange": [0, 3],
                    "MessageTypes": [ "StaticCmd" ]
                },
                "PositionWithDuration": {
                    "Name": "Move to position over time",
                    "StepRange": [0, 100],
                    "MessageTypes": [ "LinearCmd" ]
                }
            },
            "Sensor": {
                "SensorName": "Oscillate Position",
                "ValueRange": [[0, 100]],
                "MessageTypes": [ "ReadCmd", "SubscribeCmd" ]
            }
        },
        {
            "Sensor": {
                "SensorName": "Battery",
                "ValueRange": [[0, 100]],
                "MessageTypes": [ "ReadCmd", "SubscribeCmd" ]
            }
        }
    ]
}

So this keys the actuator on ActuatorType, meaning you can send StaticCmd to the same actuator with different types to control different portions of the same actuator, while also using the rigidity of the map to guarantee you can't double-define an actuator type on an actuator.

Parsing this into something usable may suck though, which is why I want to play with it a bit after I get the next version of Buttplug/Intiface out.

Yoooi0 commented 2 weeks ago

Is it really necessary to only have one of ActuatorType? Why limit it if you can just move the types inside each property, and you also can add the actuator name to keep it consistent with sensors.

"Actuator": {
    "ActuatorName": "Oscillate",
    "ActuatorProperties": [
        {
            "PropertyName": "Oscillate Speed",
            "PropertyType": "Oscillate",
            "StepRange": [0, 20],
            "MessageTypes": [ "StaticCmd" ]
        },
        {
            "PropertyName": "Oscillate Range",
            "PropertyType": "Range",
            "StepRange": [0, 3],
            "MessageTypes": [ "StaticCmd" ]
        },
        {
            "Name": "Move to position over time",
            "PropertyType": "PositionWithDuration",
            "StepRange": [0, 100],
            "MessageTypes": [ "LinearCmd" ]
        }
    ]
},

Tho there is one issue, to which actuator property is the feature sensor attached to? Since there are 3 properties in the feature and only 1 sensor.

Since we need a way to bind a motor actuator with an encoder sensor, why not make it into one thing, that just supports StaticCmd for setting the motor position, and ReadCmd for reading the encoder position. So this is the other idea I had which I dont know how doable it is in buttplug, or if it requires a lot of code change, but it feels the most "clean" to me:

{
    "DeviceName": "Device",
    "DeviceFeatures": [
        {
            "FeatureName": "Oscillator",
            "FeatureType": "Oscillate",
            "FeatureProperties": [
                {
                    "PropertyName": "Speed",
                    "PropertyType": "Percent",
                    "StepRange": [0, 20],
                    "MessageTypes": [ "StaticCmd" ]
                },
                {
                    "PropertyName": "Range",
                    "PropertyType": "Range",
                    "StepRange": [0, 3],
                    "MessageTypes": [ "StaticCmd", "ReadCmd" ]
                },
                {
                    "PropertyName": "Position",
                    "PropertyType": "Percent",
                    "ValueRange": [0, 100],
                    "MessageTypes": [ "StaticCmd", "ReadCmd" ]
                }
            ],
        },
        {
            "FeatureName": "L0 Axis",
            "FeatureType": "Position",
            "FeatureProperties": [
                {
                    "PropertyName": "Position",
                    "PropertyType": "Percent",
                    "StepRange": [0, 20],
                    "MessageTypes": [ "StaticCmd", "LinearCmd" ]
                }
            ],
        },
        {
            "FeatureName": "Normal R0 Axis",
            "FeatureType": "Angle",
            "FeatureProperties": [
                {
                    "PropertyName": "Angle",
                    "PropertyType": "Percent",
                    "StepRange": [-100, 100],
                    "MessageTypes": [ "StaticCmd", "LinearCmd" ]
                }
            ],
        },
        {
            "FeatureName": "Infinite R0 Axis",
            "FeatureType": "Rotate",
            "FeatureProperties": [
                {
                    "PropertyName": "Motor Speed",
                    "PropertyType": "Percent",
                    "StepRange": [-100, 100],
                    "MessageTypes": [ "StaticCmd", "LinearCmd" ]
                }
            ],
        },
        {
            "FeatureName": "Battery",
            "FeatureType": "Battery",
            "FeatureProperties": [
                {
                    "PropertyName": "Charge",
                    "PropertyType": "Percent",
                    "ValueRange": [0, 100],
                    "MessageTypes": [ "ReadCmd", "SubscribeCmd" ]
                }
            ]
        },
        {
            "FeatureName": "Left Vibrator",
            "FeatureType": "Vibrate",
            "FeatureProperties": [
                {
                    "PropertyName": "Strength",
                    "PropertyType": "Percent",
                    "ValueRange": [0, 100],
                    "MessageTypes": [ "StaticCmd", "ReadCmd", "SubscribeCmd" ]
                }
            ]
        }
    ]
}

The PropertyType is only really necessary if you want to reuse StaticCmd for normal values and range. Otherwise PropertyType can be removed and a new message type can be added, for example RangeCmd. Also other property types can be easily supported in the future like Integer/Double/Enum/String etc.

/me hides

blackspherefollower commented 1 week ago

Here's the cursed structure I was rotating in my mind:

My issue here is how does a client work out that "Range" on the actuator affects "Oscillation", but not "PositionWithDuration", which also conflicts with "Oscillation".

Even as a dev, there's nothing to tell me that "Range" doesn't apply to "PositionWithDuration" even if I do realise I need to make sure I pick only one of the movement commands.

Yoooi0 commented 1 week ago

My issue here is how does a client work out that "Range" on the actuator affects "Oscillation", but not "PositionWithDuration", which also conflicts with "Oscillation".

Thats true, same thing with how to map the one sensor to one of the actuator types. In my example it would probably be a separate feature, but then you still don't really know which feature property you should use.

So what about having features be directly controlled like the current actuators, but with possibility to define settings for that feature:

{
    "DeviceName": "Device",
    "DeviceFeatures": [
        {
            "FeatureName": "Oscillator",
            "FeatureType": "Oscillate",
            "StepCount": 20,
            "ValueRange": [0, 20],
            "MessageTypes": [ "StaticCmd" ],
            "FeatureSettings": [
                {
                    "SettingName": "Range",
                    "SettingType": "Range",
                    "ValueRange": [0, 20],
                    "MessageTypes": [ "StaticCmd" ]
                }
            ],
        },
        {
            "FeatureName": "Manual Position",
            "FeatureType": "Position",
            "StepCount": 100,
            "ValueRange": [0, 20],
            "MessageTypes": [ "StaticCmd", "LinearCmd" ]
        },
        {
            "FeatureName": "Motor with Encoder",
            "FeatureType": "Position",
            "StepCount": 100,
            "ValueRange": [0, 100],
            "MessageTypes": [ "StaticCmd", "ReadCmd" ]
        },
        {
            "FeatureName": "Battery Charge",
            "FeatureType": "Battery",
            "StepCount": 1000,
            "ValueRange": [0, 100],
            "MessageTypes": [ "ReadCmd", "SubscribeCmd" ]
        }
    ]
}

I think this more clearly defines the relationships, you know that FeatureSettings customize the behavior of the feature, but you drive the feature directly like before. If a feature has "StaticCmd" and "ReadCmd" message types then its a actuator+sensor pair like a motor+encoder. If a feature only has "ReadCmd"/"SubscribeCmd"message types then its a sensor.