AndunHH / spacemouse

Repository for a space mouse, which emulates a 3Dconnexion "Space Mouse Pro wireless". It is based on four joysticks with additional keys or an encoder
Other
83 stars 16 forks source link

Send HID report only with new data #34

Closed AndunHH closed 2 months ago

AndunHH commented 3 months ago

Call HID().sendreport only, when new data is available in order to reduce load on the pc.

This is implemented as a request in #33 by @simonmacarthur. Please review and test this code.

AndunHH commented 3 months ago

I checked my suggestion again. ... Sending the translation data, only when something changed is a bad idea, because the driver and the pc expects the mouse to report it's value regularly! If nothing new is reported, the models on the pc stop moving.

Therefore, I introduced the HIDUPDATERATE_MS definition to get a constant update rate.

I suggest HIDUPDATERATE_MS = 10 to send data every 10 ms = 100 Hz.

Maybe we need to adjust the sensitivity a little bit, because the full speed reporting was around 330 Hz at my configuration before and this will result in a slower movement.

The benefit with this constant update rate is, that we should get constant results, indepenent on other calculations we introduce to the mouse.

@simonmacarthur or someone else: Can you give it a try?

Additionally: I integrated the return value of suggestion 1 in #35 into send_command()

simonmacarthur commented 3 months ago

@AndunHH I've tried this new method and it works fine.

Saying that the previous version also worked ok on my machine (Win10 + latest 3DConnexion driver +Fusion360 + Trainer App), so might be an environment difference?

AndunHH commented 3 months ago

I added this last to commit to report zero values only every HIDUPDATERATESLOW_MS. This should be near the initial idea of @simonmacarthur.

As a linux users I have some troubles with not repeated rotations, when the values don't change, see https://github.com/FreeSpacenav/spacenavd/issues/108. Therefore I introduced a small jiggling on the last bit to get a "different" rotation with every value send over USB. On windows or mac (=without the spacenavd daemon on linux) you don't need it and can leave #define JIGGLEVALUES 0 to disable it.

AndunHH commented 3 months ago

I got hold off an old, used SpaceNavigator and plugged sniffed on it with wireshark:

(The SpaceNavigator is the mouse with only two buttons and not the SpaceMouse Pro, which we are imitating, but that shouldn't matter)

The SpaceNavigator is reporting translations (report id 1) and rotations (report id 2) every 8 ms, if there are non-zero values. Before stopping to transmit this value, if return to zero, the zeroes are sent three times ... I will try to imitate this behavior with our DIY approach and check if spacenavd responds to our imitation as to the real mouse.

Im also trying to figure out the Input definition of the HID descriptor, which differs and results in strange behavior. I hope to get this sorted and get rid of the jiggling part at all. See also https://github.com/FreeSpacenav/spacenavd/issues/108 for further investigation.

AndunHH commented 3 months ago

I had some interesting time with an old, used SpaceNavigator which a colleague could spare for some days. See more details in SpaceNavigator.md in 7d700db

I analyzed the data sent and could correct our HID report. There were some strange inconsistencies, with a range of motion going from -342 to +350 for example. Under ubuntu with spacenavd this led to a very small, but constant movement (all values = 4). Not sure, if this was also happening under windows. Probably not, otherwise people would have complained.

And even more interestingly, I could observe a constant 8 ms reporting of the events, if not zero. I reimplemented this as a state chart. I hope that the final approach is now simple enough, even though getting us here was not so simple.

I will also try it out by myself on thursday to check if everything is working again.

AndunHH commented 3 months ago

So, finally, I changed from absolute to relative values in the HID report descriptor to avoid the need for jiggling the values.

My tests under windows showed no impact in the trainer app from 3Dconnexion. Can you confirm this @coliss86? This is the last update in this pull request if you don't find a bug. Thanks for testing! :)

Check the discussion in spacenavd for more informations or my conclusion:

I found the problem, that in FreeCAD the model is stop moving, when the spacemouse is off-centered hold still.

My final conclusion to this is as follows: The original Space Navigator reports it's data as relative positions. The original Space Navigator is very sensitive and is jiggling a lot i.e. the same value is only send repeatedly very for some milli-seconds.

My emulated "SpaceMouse Pro Wireless (cabled)" (or at least our inherited) hid report descriptor reports absolute values Our emulation is very sturdy and if you hold it in position, the same value are easily calculate for a second.

When using spacenavd and the simple example, cube or even in FreeCAD:

  • Relative reports are evaluated with every event, even if they are the same as before.
  • Absolute reports are only evaluated, if they differ from the previous report. This is merely visible with the SpaceNavigator, but is very annoying for our emulation, as it is reporting same values very often. I didn't figured out, if this dropping of events is done by spacenavd or linux itself...

Solution: Change our emulated mouse to Relative Positions, even if this is not "up to date". But it avoids the necessity to jiggle the values.

coliss86 commented 3 months ago

What a nice work ! I made 2 remarks but my mouse isn't working for the moment thus I can not test it :'(

coliss86 commented 2 months ago

I finally tested it: it works without problems in fusion360 but in the 3d connexion viewer, there are some skip in the rotation or translation. What do you think of just adding a delay(4) between each loop instead of having this code so complex ?

AndunHH commented 2 months ago

Thanks for your tests and your review comments!

I'm in favor of the complex code, because no we ensure an update rate of exactly 125 packages send per second, independent of future improvements (=more processor load). If we just use a delay(4) in between sending the data the system will receive e.g. 125 data per second today. If someone is activating a more complex logic (for example the encoders with some future magic) the system will take longer for one loop() leading to less hid updates per second. Which would make the mouse slower, because it has to calculate more and sends less updates.

I also did some final tests on a windows machine and tried the 3d connexion viewer and OnShape. I couldn't reproduce skipped steps. I'm not sure how to debug this... Is it a deal-breaker, which we need to fix now?

Can we merge this branch into main?

coliss86 commented 2 months ago

I understand your concern, sure you can merge :)

I don't think the problem I had is a deal breaker as it works in fusion. I suspect the 3d connexion viewer buggy.