SlimeVR-Server
SlimeVR-Server copied to clipboard
Fix acceleration axis and leg tweaks
Fixes #1006
This PR fixes the axis for leg tweaks! However, an additional update of thresholds may be required.
@ImUrX, please test it in standard mode with leg tweaks like skating correction.
I don't have slimes for testing.
This won't affect skating-correction since the magnitude of acceleration is not being changed.
Not really. Some code uses only the two components, like Vector3(footDif.x, 0f, footDif.z) . It uses x and z components, which correspond to trackers to original (x y) components in this PR and trackers (y z) in the current main branch. z in the tracker coordinates corresponds to the gravity direction. That is what the fix is about.
private fun correctUnlockedFootTracker(footPosition: Vector3, previousFootPosition: Vector3, previousFootPositionCorrected: Vector3, footVelocity: Vector3, framesUnlocked: Int): Vector3 {
var newFootPosition = footPosition
var footDif = footPosition - previousFootPositionCorrected
footDif = Vector3(footDif.x, 0f, footDif.z)
This won't affect skating-correction since the magnitude of acceleration is not being changed.
Not really. Some code uses only the two components, like
Vector3(footDif.x, 0f, footDif.z). It usesxandzcomponents, which correspond to trackers to original (xy) components in this PR and trackers (yz) in the currentmainbranch.zin the tracker coordinates corresponds to the gravity direction. That is what the fix is about.private fun correctUnlockedFootTracker(footPosition: Vector3, previousFootPosition: Vector3, previousFootPositionCorrected: Vector3, footVelocity: Vector3, framesUnlocked: Int): Vector3 { var newFootPosition = footPosition var footDif = footPosition - previousFootPositionCorrected footDif = Vector3(footDif.x, 0f, footDif.z)
That is a positional correction, the acceleration is not used like that, I just checked. Mocap mode does use the acceleration direction though so this will help that!
@ImUrX is anything else required for this PR to be merged?
You need to wait for more reviewers
I though that two reviwers are enough. Should we ping someone to make a review?
@Stermere id don't know if the fusion does the order right, but, at least, they in the right order during packet formation.
If you like I could take the original AXES_OFFSET and apply it both to rotation and acceleration. So, it will be defined by mostly by software, not a math.
On the other hand, if the bug is in the fustion, than the firmware has to be fixed instead of tuning the server for the firmware. At least, we could define a new version of protocol. The reason is because slime wants to be a kind of opensource standard for VR.
The current acceleratoin offsets is defenetely wrong. Some code may work better now because it was tuned for the current wrong offsets. That could work for now but it can be so for a robust code evolution.
We should check firmware for all main IMUs and make sure that it sends acceleration in the same axises, I guess.
@Eirenliel We don't use the Slime firmware and I am not sure to fix it. I believe the bug is both in leg tweaking and in firmware in the way they are cancel each other in one way or another. The reason is that the software evolved with the firmware bug present. On the other hand, the bug constraint further progress in leg tweaking and etc because people work with stuff that doesn't really mean what it says it means. So the software evolves according to Darwin laws instead of common sense and math.
Can we merge this PR before the firmware fix? I can rebase it and simplify it by explicitly applied the same transformation like for rotation.
both bugs may be on the server side as well.
@9il I am interested in testing this, I had a friend do some research into it. Where is this github at that contains this fix? Or has this already been fixed and the issue is not closed?
Hi @Shade-emry, there are currently other issues as mentioned which should be resolved first.
Hi folks. I can update this PR if you confirm it that nothing else is blocking it. We don' use slime firmware, and can't fix it (if it is requires fix which is not clear).
This fix loos correct. We should confirm that it doesn't break legtweaks, @Stermere @ButterscotchV and we can merge it after 0.14.0 release
rebased