lerobot icon indicating copy to clipboard operation
lerobot copied to clipboard

feat(teleop): thread-safe keyboard teleop implementation

Open imstevenpmwork opened this issue 8 months ago • 3 comments

What this does

Our current teleop implementation is not thread safe. This adds the necessary locks + an example file This PR is meant to be Squashed Merged

imstevenpmwork avatar Mar 17 '25 14:03 imstevenpmwork

~~I don't think this is needed as pynput.keyboard.Listener operates as a passive listener (so it should never conflict with other programs using the keyboard)~~

EDIT: After check we might have race conditions when reading that dict. In that case, I would much prefer using a queue.Queue as it's suited exactly for that purpose rather than a lock. Could you make the changes?

aliberts avatar Mar 20 '25 15:03 aliberts

Using a synchronized queue is indeed another valid implementation 👍🏻 IMO: Locks: ✅ Low overhead ❌ Manual synchronization Queues: ✅ Auto synchronization & event-driven ❌ Higher overhead, memory complexity increases linearly with events & slightly more complicated code

But honestly for this use-case, both work perfectly okay 😄 Here's the PR with the implementation using Queue: https://github.com/huggingface/lerobot/pull/886

imstevenpmwork avatar Mar 21 '25 10:03 imstevenpmwork

Is this ready for merging?

imstevenpmwork avatar Mar 23 '25 00:03 imstevenpmwork