Add Velocity calculation logic and GUI toggle (end-to-end MVP).#1753
Add Velocity calculation logic and GUI toggle (end-to-end MVP).#1753M1DNYT3 wants to merge 11 commits intoSlimeVR:mainfrom
Conversation
b502b9c to
b83d03d
Compare
|
Force pushes in history are changes made by spotless. |
|
At some point, I had a conversation with @ButterscotchV in DMs, and there was a recommendation expressed to ship the whole feature (Implying a toggle, nothing overkill) in one PR. The description is edited accordingly to reflect each affected part of the code. |
… latest definition from the Driver's main branch.
…ce, Replace allowVelocity with hasVelocity in ProtobufBridge.
…#1811 # Conflicts: # server/core/src/main/java/dev/slimevr/protocol/rpc/settings/RPCSettingsBuilder.kt
� Conflicts: � server/core/src/main/java/dev/slimevr/protocol/rpc/settings/RPCSettingsBuilder.kt
|
I had some additional feedback from @ButterscotchV in DMs, addressed it all + rebased to the latest main + also did some changes of my own. Feedback changes:
My own improvement: |
…timing fix, non-nullable writeTrackerUpdate param
|
Review changes were shipped and tested E2E with SteamVR. One thing to note. Jitter is still noticeable. I may think of a one-euro filter or some velocity smoothing added, but I suggest that to be a separate PR once this one is shipped. |
ButterscotchV
left a comment
There was a problem hiding this comment.
Looks fine, SolarXR needs to be merged/fixed before this PR.
| // Disables derived velocity for all trackers. Driver zeroes out velocity if nothing is returned in protobuf message. | ||
| var sendDerivedVelocity: Boolean = false | ||
|
|
||
| fun updateTrackersVelocityPolicy() { |
There was a problem hiding this comment.
"policy" is a wording not used in the rest of the codebase and was confusing for me on the first read. This should probably be named updateTrackersVelocitySettings, like in ResetsConfig.
| /** | ||
| * Updates the derived velocity of the tracker by differentiating position over time. | ||
| * | ||
| * This method enforces the [allowVelocity] policy and checks for valid position data before | ||
| * proceeding. If conditions are met, it calculates velocity based on the displacement since the | ||
| * last update, applying a sanity check on the time delta to filter out noise and ensure data stability. | ||
| * | ||
| */ |
There was a problem hiding this comment.
Could be shortened
| /** | |
| * Updates the derived velocity of the tracker by differentiating position over time. | |
| * | |
| * This method enforces the [allowVelocity] policy and checks for valid position data before | |
| * proceeding. If conditions are met, it calculates velocity based on the displacement since the | |
| * last update, applying a sanity check on the time delta to filter out noise and ensure data stability. | |
| * | |
| */ | |
| /** | |
| * If derived velocity is enabled, updates the derived velocity of the tracker, calculating it based on the position displacement | |
| * since the last update, applying a sanity check on the time delta to filter out noise and ensure data stability. | |
| * | |
| */ |
This PR completes velocity transmission end-to-end, adding RPC settings and a GUI toggle on top of the foundation from #1754.
What's added:
Toggle's GUI Path:
Settings > General > Tracker Settings > Toggle is at the very bottom of the section.
Technicalities:
Most of the line count (~400) comes from regenerated ProtobufMessages.java. The handwritten change itself is small.
Depends on SlimeVR/SolarXR-Protocol#202, should be merged only after the submodule is updated to the tracked HEAD.
Addresses the issue: #25