Skip to content
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

Step Range / Step Limit Confusion #172

Open
Eusth opened this issue Feb 9, 2025 · 3 comments
Open

Step Range / Step Limit Confusion #172

Eusth opened this issue Feb 9, 2025 · 3 comments

Comments

@Eusth
Copy link

Eusth commented Feb 9, 2025

Problem

The step limit range slider is very confusing and possibly useless as of right now.

In my understanding, this is how the system currently works:

  • step_limit controls the step count reported to clients, but they are free to honor or ignore it.
  • step_range controls the actual scaling of the scalar values, i.e. [0.0..1.0] is mapped to [step_range.0..step_range.1].

As it stands now, Intiface Central only sets step_limit and uses step_range to keep track of the absolute min / max values of the respective actuator.
...Meaning that the setting is pretty much useless, and may well be the cause of ... unpleasant surprises in the nether regions, when people think they'd turned their super strong device down a notch, only to be buzzed into oblivion.

Possible Solutions

If my understanding is indeed correct:

  • a.) I think the UI should at least be modifying step_range instead of step_limit. (Although that would be kind of tricky to do in a backward compatible way.)
  • b.) But more correctly, I believe both values should be controlled by the range slider and kept in sync, since that's probably the thing users want to achieve in most cases, i.e. define which subrange of the device be usable and have this also reflected in the step count.
    However, this would require that the min / max values are tracked elsewhere. Either in an Intiface-only config, or by fetching the base values from the Device Configuration Manager (although there are not getters yet, and I don't know if there should be.)
@qdot
Copy link
Contributor

qdot commented Feb 9, 2025

I think we do what you want already, and I'm not sure where your understanding of the system came from, so I'll run through the design real quick here (with the cavaet that I am jetlagged af right now, so if this comes out weird or confusing or sounds wrong, just lemme know).

The way things should work at the moment is:

  • step_range is the full range of settings the device allows. For instance, on Lovense device actuators, it'll always be [0,20]. This will never be modifiable because it's our base source of truth about the device capabilities, and exists in our base device config database.
  • step_limit is a subset of the values of step_range used for device speed regulation. This only exists in our user configs, which are overlaid on top of the device configs.
  • step_count is what a client application sees when the device is enumerated. As of right now, this is a single value. For an unmodified Lovense device, that'd be set to 20, because it has 20 steps.

Let's say you want to set a Lovense device to only have a vibration speed range between 6-10 (we also always keep 0 to mean "stop"). You'd set step_limit to [6, 10] (it's a multislider). This is set in intiface central, and stored in the user config file.

When Buttplug starts up, it loads the base config database, which just has all of our default definitions in it. Then it loads the user config file, which has extra definitions keyed on device name/identifier/address triplets. When you connect your Lovense device that you've made the step_limit changes to, it identifies the device as the user config since it'll match the triplet, and sets our version of the device to be controlled in that range. Then, when we look up the device to enumerate it to the client, we see that it has a range of 5, so the client gets a step_count of 5 (step_count = max - min + 1, so that 1 / step_count gives you the multiplier value you need to know how to set your actuator value).

The client sets actuation values (i.e. ScalarCmd in this case) on a basis of 0.0-1.0 currently (this is slated to change in our next API version to just actually be an integer, using floats was overkill), but within the 0.0-1.0 range we'll only really have 6 set points (0 - 0, 0.2 - 6, 0.4 - 7, 0.6 - 8, 0.8 - 9, 1.0 - 10), with all values in between ceiling()'d to round up to the next valid step, so someone setting 0.01 actually goes to 0.2.

The complexity here is due to a combination of this design being built somewhat on the fly over the past versions of the protocol, as well as striving to only send required minimum info to the client.

If the system isn't working this way, then its a bug, though I think we have tests for this down at the buttplug level.

@Eusth
Copy link
Author

Eusth commented Feb 9, 2025

Thanks for the detailed explanation (and for battling your jetlag -- good recovery!) That clears things up a bit, and is also how I expected the system to work.

We might have a bug at hand here, though, since I'm not seeing that effect with my device(s). (I've only tested a Lovense device so far, but I'll give my other devices a try, too.)

My (admittedly wobbly) understanding comes from skimming through the source code. More precisely:

  • The fact that only step_range seems to be considered for turning the float into an u32. (ActuatorCommandManager)
    In order to function as you described (i.e. translate the normalized value to the subrange), I would expect something roughly like:
    pub fn update(&self, value: &(f64, bool)) -> Option<(u32, bool)> {
      let mut result = None;
      let range_start = *self.actuator.step_limit().start();
      let range = self.actuator.step_limit().end() - range_start;
      let scalar_modifier = value.0 * range as f64;
      let scalar = if scalar_modifier < 0.0001 {
        0
      } else {
        // When calculating speeds, round up. This follows how we calculated
        // things in buttplug-js and buttplug-csharp, so it's more for history
        // than anything, but it's what users will expect.
        ((scalar_modifier + range_start as f64).ceil() as u32).clamp(
          *self.actuator.step_range().start(),
          *self.actuator.step_range().end(),
        )
      };
  • The fact that step_limit seems to be only used to surface the step_count to the clients. (According to a quick search.

* This is my first time looking into the server code, so chances are I'm missing something.
* I only looked at the master branch. I believe v4 may have already fixed the scaling issue since clients pretty much have to consider step_count. Although I'm not sure about the offset.

@qdot
Copy link
Contributor

qdot commented Feb 9, 2025

Ok this definitely could be a bug. I'll see about taking a look this week, as well as checking our test coverage to see if my claim about having tests for this is true (it may very well have been missed).

And yeah, v4 removes a LOT of this complexity and I can't wait to get it out into the world, just gotta find time to finish it up. :|

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants