Reorganize server.py, autodetect sensor count from MCU

[joshhead]

Rewrite most of server.py to centralize the main loop in one place (see line 673).

• Detect number of sensors on serial connect
• Backpressure for sensor values broadcasts (waits for sensor values to send before polling serial again)
• High-level connect/reconnect and send/receive loops are in one place in the code
• Clean shutdown without printing errors in the terminal
• Log only one set of thresholds when doing a threshold read to and write from microcontroller on serial connect
• Add new SERVE_STATIC_FRONTEND_FILES flag, to control a subset of what NO_SERIAL would change previously
• Conform to google's python style guide
• Prefer coroutines to threads
• Reduce number of global variables
• ProfileHandler handles profiles and file IO but does not initiate serial broadcasts
• NO_SERIAL logic is in its own class

[dominickp]

I hope we can look into this or other performance improvements because running 8 sensors is currently a miserable experience for me.

I really think there is too much data being sent to the computer. If the PC can't keep up and process the change in value in a certain amount of time, it should be dropped. It isn't useful to queue up 10 seconds of inputs and play them back all delayed. Like a backup camera in a car, I'd like the quality to just drop to keep the information as realtime as possible. I think dropping the "resolution" or sample rate somehow would be useful here too.

We obviously want the joystick button presses to be as instantaneous as possible. But the sensor value readings are much less important and if we had 1/10th the resolution but it was real-time vs 100% of the resolution and its delayed by 10 seconds, the former is way more useful. Hope this makes sense.

[dominickp]

I also wonder if the performance updates to CPython in 3.11 could be helpful here too, thinking back to how the sleeps seem to cause this backup in queuing on some slower Windows machines...

[joshhead]

@dominickp IIRC there were two remaining blockers to merging. One, it's difficult to review because of the large scope of the changes. Two, it hasn't been independently verified. If you were able to check out this branch and confirm whether it works well for you, I think it would help move this forward.

For me, it very noticeably improved responsiveness with 8 sensors on a Teensy 2.0 (same ATMega32U4 MCU as the Arduino Pro Micro).

I haven't touched this in a while, and I see there are some conflicts with the latest updates. I can look at resolving those if there's still interest in getting this in.

[teejusb]

I think it might worthwhile to break this into a couple smaller PRs. I think my main struggle with reviewing is understanding the vast swaths of changes happening to server.py. I think the auto detection of sensor count can happen its own PR, and then the reorganization can be separate if that's possible. I'm unsure what the reorganization is achieving too so if the PR could detail the pros and cons that would be very useful, (maybe inline as well).

[joshhead]

I think the changes will be difficult to split up. A better approach may be to start over, moving forward in smaller, self-contained chunks.

The biggest features I want to keep are:

• Reduce latency of inputs showing in UI
• Clean shutdown without errors/hanging
• Clean connect/disconnect/reconnect of MCU without hanging

Autodetecting # of sensors is also nice, but it's easier to add after fixing some other issues.

It's been a long time since I worked on this but as I recall, I started on this set of changes because I thought the organization of the send/receive code could be improved.

In the upstream code, sending and receiving happen in two different threads, which share one Serial instance. I don't think putting them in two threads is necessary, and makes the code more difficult to follow. Also, there is some mixing of real threads with async.io coroutines which IIRC caused some difficult to understand bugs.

I would rather rewrite the threading code than fix the threading bugs then throw the fixes out for a rewrite, but that's the mindset that resulted in a change that's too big to review. 😌

[joshhead]

The last thing I did was reformat the code to match Google's coding style. I'd probably start with that this time around even though it will make it harder to merge this branch.

[teejusb]

I'm down to talk about how we want to move forward. I think the features you've suggested seem pretty good and I think doing it in smaller chunks will make the review cycle a lot faster. I think my biggest struggle was looking at this PR and finding it hard to follow exactly what was happening (as it was a full rewrite) and I didn't want it to be in a state where I myself didn't understand how the code worked.