-
-
Notifications
You must be signed in to change notification settings - Fork 65
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rethink how Client Device Actuators are handled/exposed #552
Comments
Some examples/explanation for the proposal. Current sample device{
"name": "Sample Device",
"messages": {
"ScalarCmd": [
{
"ActuatorType": "Vibrate"
},
{
"ActuatorType": "Position"
}
],
"RotateCmd": [
{
"ActuatorType": "Rotate"
}
],
"LinearCmd": [
{
"ActuatorType": "Position"
}
],
"SensorReadCmd": [
{
"SensorType": "Battery"
}
],
"SensorSubscribeCmd": [
{
"SensorType": "Pressure"
},
{
"SensorType": "Battery"
}
]
}
} Given the above sample device there are a few issues:
Proposed sample device{
"name": "Sample Device",
"actuators": [
{
"ActuatorType": "Vibrate",
"MessageTypes": [
"ScalarCmd"
]
},
{
"ActuatorType": "Rotate",
"MessageTypes": [
"RotateCmd"
]
},
{
"ActuatorType": "Position",
"MessageTypes": [
"ScalarCmd", "LinearCmd"
]
}
],
"sensors": [
{
"SensorType": "Battery",
"MessageTypes": [
"Read",
"Subscribe"
]
},
{
"SensorType": "Pressure",
"MessageTypes": [
"Subscribe"
]
}
]
} Given the above proposed device:
Possible issues:
It can also be expanded to raw commands: "endpoints": [
{
"Name": "endpoint"
"MessageTypes": [
"Read",
"Write"
"Subscribe"
]
}
] instead of having 3 separate lists for read/write/subscribe endpoint commands. |
A solution for part of the rotate issue in terms of a Client API implementation: The only difference between ScalarCmd Rotate and RotateCmd is that RotateCmd has a 2d direction (clockwise/counter-clockwise). Under a first-class actuator system, a Rotate actuator can just have a "HasDirection" check. If it's a actuator that maps to ScalarCmd rotate, that's false, if it's RotateCmd, that's true. Then it's up to the user to check, but the Rotate method of that actuator could take (-1.0, 1.0) and throw an error if x < 0 when it's a ScalarCmd Rotate. In the protocol, v4 will probably fix this by getting rid of RotateCmd and adding negatives in the definable ranges for ScalarCmd, I just wasn't quite ready to make that jump yet when I put out v3. |
Yea, I also thought about removing So |
Make a system where actuators can be addressed as a whole (i.e. all vibrators, all rotators, etc) or individually. Expect that the server will handle aggregation at a later point. Addresses #552
Make a system where actuators can be addressed as a whole (i.e. all vibrators, all rotators, etc) or individually. Expect that the server will handle aggregation at a later point. Addresses #552
Annnnd 9 months later, finally starting to think through this for implementation, and I have a small tweak that I think should cover everything. Devices will be trees of features, each of which can have both actuators AND sensors:
For the design in this issue so far, we got through the issue of "multiple ways to command the same actuator on a device, by making the actuator first class", which gets us most of the way there. What happens when we have a motor we can control, that we also have an encoder for? In v3, as well as in the above design, while we at least collapse context for actuators, we don't have any way to programmatically share context between actuators/sensors. This is an issue in Buttplug right now. With the KGoal Boost, we get two readings: Raw and calibrated, from the device. These are two readings for the same feature, but that's not obvious. The above solution won't quite fix this, because I still can't define two different SensorRead's on the same feature. Under a feature tree, we could define multiple sensor read/sub messages depending on what version of the data a user wants, but also programmatically show that they're coming from the same basic thing. I realize we don't usually expect people to close control loops via buttplug so the motor/encoder example is a bit much, but we may run into other situations where this is handy. For 98% of Buttplug developers, this is going to be too much info and they won't use it. However, for anyone developing a client, this is just an extra level of iteration they can abstract away from the user and still present things as groups of vibrators, rotators, whatever. If people really DO want to get fancy and see what groupings are, they have the option if the feature tree is exposed in whatever the client library is. For 98% of Buttplug devices, this is going to be pretty much useless too, since they're just a single vibrator. It'll just be an extra layer to unpack. I don't really feel like that's a big deal either. Our goal is wide support for devices and controlling them, so at some point we've gotta have complexity. For the 2% of either of the above, where things get complicated (which will probably continue to grow as toys get more complex), I think this should cover us. What I've said so far here only affects device enumeration, though. I'm still figuring out how I want command structure to work around this. We can make every feature and every actuator/sensor under a feature have unique IDs, or we can require sending both feature and actuator/sensor id with a message (i.e. to differentiate when we have 2 SensorRead's with different units or whatever). @Yoooi0 @blackspherefollower lemme know your thoughts. |
I think you would not define two sensor Read message types for one sensor, but rather have two separate sensors. {
"sensors": [
{
"Name": "Battery (%)"
"SensorType": "Battery",
"MessageTypes": [ "Read", "Subscribe" ]
},
{
"Name": "Battery (Raw)"
"SensorType": "Battery",
"MessageTypes": [ "Read", "Subscribe" ]
},
]
}
If I'm understanding correctly, under the feature tree you would want to have one "calibrated" and one "raw" feature, each feature with one sensor with one Read message type? If we assume we can have multiple sensors with the same SensorType in the list like above, then features only really group the sensors and actuators but do not provide any benefit from api or client perspective. var motor = device.Actuators[0];
var calibratedSensor = device.Sensors[0];
var rawSensor = device.Sensors[1]; to var motor = device.Features[???].Actuators[0];
var calibratedSensor = device.Features["Calibrated"].Sensors[0];
var rawSensor = device.Features["Raw"].Sensors[0]; In both cases client has to know the specific device it is working on to know what each feature/sensor/actuator names or indexes actually do. So you can arrange the data in many ways all it really changes is how you access it in the client. {
"actuators": [
{
"ActuatorType": "Position",
"MessageTypes": [ "ScalarCmd", "LinearCmd" ],
"Sensors": [
{
"SensorType": "Unknown",
"MessageTypes": [ "Read" ]
}
]
}
]
} var motor = device.Actuators[0];
var calibratedSensor = motor.Sensors[0];
var rawSensor = motor.Sensors[1]; What if the features are its own separate thing but they only include indexes that map to the actuators/sensors lists: {
"features": [
"raw": {
"Actuators": [0],
"Sensors": [0]
},
"calibrated": {
"Actuators": [0],
"Sensors": [1]
}
]
} Then the api implementation could have some helper methods that gives the client sensors/actuators based on the feature it wants.
That seems like it could be prone to errors, in original solution the ID was just the index in the list. |
@Yoooi0 You're reading slightly too far into my example. The Boost has 2 ways to get the same reading, but that's also currently the only toy that does that. So no, don't wanna take the raw/calibrated and run with it everywhere. It was just a way to talk about getting different units from the same feature. That's the major issue with our sensor system in general. If this were... pretty much any other type of engineering, we could figure out what units we're working under and standardize to that. No such luck working with sex toys. We're not sure how we're gonna get things like pressure or accelerometer data. That's why I brought this up. And yeah, there would definitely be names/descriptions in the features as well as actuators/sensors. So we could say a feature is "motor", positional actuator "Moves motor to position", sensor "Tracks motor position (with some sort of units)", etc... |
Oh yea, I was just using that in each example, didnt mean that each device would have raw/calibrated stuff.
If actuators/sensor will have names and descriptions then features dont add much, they just nest everything down one more level. They also increase the complexity of each command and library implementation. Just adding multiple sensors/actuators with the same type but different name and description to the list seems like an easy solution. Such grouping by feature would be nice from client perspective but I think internally they can easily just be a separate thing that contain indexes that point to actual sensors/actuators lists (I guess this can be prone to errors), the library should then implement all the necessary helper methods. Also for 99.99% of devices features will add an additional json level (also need a default feature name?). If features are its own thing then they would be just an empty array. |
Ok having sat with this for a week, I'm warming up to this idea. It's easy to ignore for everyone who doesn't want it and it's easy to leave out for most of the devices that won't need it. How we define these in the device config is gonna be... interesting, but I was considering giving every device/actuator/sensor its own UUID anyways (which we probably won't ship to the client but it'll make matching easier in the server situation and will just be built in if I ever end up moving device config to sqlite). |
Yup that's my thinking too.
Actually if the library implementation is able to use the actuator/sensor UUID then it can be another way to send commands. In such case a Sensor/Actuator class implementation would not need to know its owning device index and its index in the list and could just send commands itself using its UUID. |
Yeah, trying to decide on that. Buttplug has this whole thing of supposedly not letting the client know what toys a person might be using, but we never actually followed it due to the device names not being easily changeable. Also not quite sure how much I want to dive into both rebuilding the message formats AND the references, mostly worried about this being another Forever Project. We'll see how it goes once I get started on it. |
Random thought: a device could realistically allow both reading and commanding, say, a position, so the differentiation of actuators from sensors at the protocol level seems incidental. If you were to merge those, the protocol could look like {
"DeviceName": "Sample Device",
"DeviceFeatures": [
{
"FeatureName": "Battery (%)",
"FeatureType": "Battery",
"ValueRange": [[ 0, 100 ]],
"MessageTypes": [ "Read", "Subscribe" ]
},
{
"FeatureName": "Battery (Raw)",
"FeatureType": "Battery",
"ValueRange": [[ 0, 65535 ]],
"MessageTypes": [ "Read", "Subscribe" ]
},
{
"FeatureName": "Inserted Vibrator",
"FeatureType": "Vibrate",
"StepCount": 20,
"MessageTypes": [ "Scalar" ]
},
{
"FeatureName": "Insertion Depth",
"FeatureType": "Position",
"ValueRange": [[ 0, 20 ]],
"MessageTypes": [ "Scalar", "Linear", "Read" ]
},
{
"FeatureName": "Firmware",
"FeatureType": "Unknown",
"MessageTypes": [ "RawRead", "RawWrite" ]
}
]
} and I don't think that feels over-engineered. |
Ok, well, tackling this yet again in the new year. Good news is, every time I revisit this, the more I'm becoming ok with the simplifications everyone is recommending. Both ID'ing by actuator alone AND simplifying message configurations to just be features that can both read and write are sounding good on their face, both from the perspective of message references and configuration. The one things missing now is how to actually express those configs, because the json files are just getting ridiculous at this point. I'm looking at my sqlite branch from last April and considering continuing implementation on that, because this is now actively blocking per-actuator limit configs. |
Ok, well, after a year of fucking around trying to do this in a RDBMS and learning a bunch about sqlite then chucking it all, I'm implementing this in the old JSON format now. Still looks mostly like @CAD97's idea with a slight variation: {
"DeviceName": "Sample Device",
"DeviceFeatures": [
{
"FeatureName": "Battery (%)",
"FeatureType": "Battery",
"Sensor": {
"ValueRange": [[ 0, 100 ]],
"MessageTypes": [ "SensorReadCmd" ]
}
},
{
"FeatureName": "Inserted Vibrator",
"FeatureType": "Vibrate",
"Actuator": {
"StepCount": 20,
"MessageTypes": [ "ScalarCmd" ]
}
},
{
"FeatureName": "Trackable Linear Motor",
"FeatureType": "PositionWithDuration",
"Actuator": {
"StepCount": 100,
"MessageTypes": [ "LinearCmd" ]
},
"Sensor": {
"ValueRange": [[0, 100]],
"MessageTypes": ["SensorReadCmd", "SensorSubscribeCmd"]
}
},
{
"FeatureName": "Raw I/O",
"FeatureType": "Raw",
"Raw": {
"Endpoints": ["rx", "tx"],
"MessageTypes": [ "RawReadCmd", "RawWriteCmd", "RawSubscribeCmd"]
}
}
] So instead of everything existing on the feature object root, there's now 3 subtypes:
I really don't like having to consider a whole bunch of nullable fields and possibly invalid messages by storing everything on root, but I also get that descriptions on everything is overkill, so this feels like a decent tradeoff. Right now I'm rebuilding the device config system to both deal with this layout as well as handle user config serialization in a way that isn't shit. I'll probably ship that as part of the Buttplug v7 line while starting work on the new message system, which I may try to beta with some client developers (all 3 or 4 of them that exist :| ) before shoving the new message spec out this time. |
I think that should work, although few things that crossed my mind:
|
Ok, so, questions 1 and 3 overlap a bit. If we assume a feature will have no more than 1 actuator/sensor/raw, then we'd use the feature index, because no messages will overlap on actuators/sensors (raw is extremely special/rare/weird. It'll only ever be on its own feature, will only ever be one of them. I honestly doubt most client devs will even implement it). The valid message types for actuator/sensor/raw are all disjoint subsets of the device message set. If we do have multiple adapters/sensors, then yeah, shit gets weird. I've been trying and failing to come up with a situation where there'd be multiple actuators on a feature, and the only situation I can think of right now where we'd have multiple sensors is if we had, say, a motor with position and temperature output or something. At that point I feel like we're starting to operate outside of the simple stuff this library was meant for anyways (i.e. if you're trying to close a control loop and monitor how it affects motor temperature through buttplug, maybe you should be considering an Actual Controls Library :) ). Agreed on PositionWithDuration. Just using ScalarCmd/LinearCmd does work. Thanks for that callout.
|
Also cc @blackspherefollower because they're the one with all the devices. Any ideas about single versus multiple actuators/sensors per feature? |
I think the only cases where multiple actuators or sensors for the "same" feature make sense are either:
The goal should be to hopefully simplify the protocol by flattening it. This is at its core an Io(s)T protocol, and individual devices aren't likely to be complicated enough that just listing it as two separate related features in the array would be a problem. A "feature" is the user-meaningful unit of address here. (I also don't really see |
I'm pretty jetlagged right now, but it took me a second to work out that feature is effectively just the superclass of actuator/sensor and not necessarily a physical part (I've got a device that's both suction and insertable connected by flexible cable and both ends have separate heating elements: the heaters would be a separate feature rather than the insertable element having both heater and vibrator as actuators). Being able to associate a sensor with an actuator on the same physical part of a device could be handy, but I think it's rare enough for devices to have sensors and multiple physical parts that the description will suffice. To extend the talk about Position via LinearCmd/ScalarCmd, we also talked about exposing Oscillate/OscillateWithRange on positional devices so that end users didn't have to reinvent the wheel each time they needed to set a positional device to a speed. If these are considered as a different feature, then a way to know they affect the same actuator is going to be needed (otherwise clients will just iterate over all the features and override commands they just applied). |
@blackspherefollower Where did the "OscillateWithRange" discussion happen? I can't find anything on discord history nor here, so I don't remember what the context was. Also, I should say: This isn't going to be set in stone quite yet. Unlike prior releases, my plan for now is to implement this format in our device configs and inside the library, so we can at least make sure it fits internally and that all of our current device configs conform to it. I'd like to actually release another few versions of the library using the current spec while it sits on top of this configuration and we figure things out. Once we're sure this is going to cover our needs then I'll put the messages on top of it and update the spec (and I may include some way to beta the new message versions during that time) |
The important bit was to do with the idea that positional linear devices could have ScalarCmd Oscillation support exposed and implemented in the buttplug server; this would mean that the same physical actuator might be controlled by either ScarlarCmd or LinearCmd. It'd be important to let the client know not to send both commands because one will override the other. This last came up in the conversation we had around the depth control on the Lovense Solace (I've copied it into #611 (comment) ); it's only mentioned in passing though.
That sounds good. Technically the server could just start supporting the new protocol and without it being documented or exposed by clients the downgrade logic should just make this invisible to existing users (expect of course the overhead of implementing the downgrades upfront). I'd be hitting the engine anyway, so maybe this just goes in a beta branch with CI enabled? Lots of ways to skin this cat. |
This is exactly the plan. Right now I'm mostly worried about how we store device capabilities. Next, we worry about how we communicate them, then finally, how that looks in relation to device command/reading messages going forward. I'm implementing the config updates now, as well as the internal storage of device configurations to work with the current message system. This will probably end up in me making new top level messages that I'll use as conversion endpoints, but just won't expose in the protocol until we figure out how everything is going to work. The other thing I don't want happening here is the config formats directly affecting the API. I know there's client authors here who want a simpler interface to make client implementation easier, but I don't want that affecting the project as a whole (either my ability to make the backend work as needed, or the fact that client APIs should cater to their users and don't need to be direct 1:1 mappings of the low level protocol), so I'm trying to balance needs as I can. All that said: I still have no fucking clue what to do about the goddamn solace. :| |
Ok, finally getting this into the spec/code. Here's the first draft of the changelog for the v4 spec. Some of this (including everything pertaining to this bug) has already been implemented in what will be the server side of Buttplug v8.1, coming out sometime soon. I'll be shipping the next version of Intiface Engine (not central, because this is gonna require a ton of UI updates as well as updating the dart library to v4) with an unstable flag that can turn on v4 spec usage capabilities. I'm still not done actually implementing all of the changes listed (and they may not all make it in), but it'll give people a chance to use Engine to test new client code on v4 as development continues. https://spec-v4--docs-buttplug-io.netlify.app/docs/spec/changelog |
What would be
Maybe
I think Also, TCode lube works like any other axis, so technically it would accept both
So I'm assuming |
Combo of ActuatorType and SensorType.
Oh, hmm. ConstantCmd might work, yeah. Interpolated might work too. I'll mull that over.
Yeah. BangCmd is a nod to Max/MSP and PureData, which uses "bang" to denote a single event, but that's a little obscure. ActionCmd ain't bad.
Since TCode and Hismith both have lube injectors out now, might be time for a Lubrication type.
StaticCmd range will be defined by... Oh god fucking damnit I only send steps not allowable range in the enumeration info for Scalar/Static/whatever we're calling it. Ok well that's why I posted this, I knew I was gonna miss stuff. :) I'd considered having separate "Rotate" and "RotateWithDirection" FeatureTypes, but I'm not sure how much I want to hinge ranges on client developers having to know to correlate FeatureType with whatever a range may be. It may end up being the solution though, I just want to actually think about it a bit first. |
Adding a Also I think What about
I think it has to be removed. |
Hmm. Yeah. After I get the next version of engine/central out I'll play with this and see how it feels. BTW, in terms of development: I would like to let client developers like you play with v4 before we just sign off and ship it. For now, the plan is to add an extra argument to intiface engine to allow v4 connections, so you can start at least planning for/playing with the message formats and we can see how they work out. This functionality may find its way up into intiface central, but there's a lot of weirdness around the devices tab and internal communications that I don't want blocking the next release.
Agh this is why I was worried about splitting discussions across issues. See #565 (comment) which necessitates StaticCmd + Type as a resolution to the incredibly annoying Solace Range+Speed problem. |
From @Yoooi0 on discord:
The text was updated successfully, but these errors were encountered: