Add bitdepth command and bitdepth=12 support to Flex driver #146
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Turns out the Flex firmware does not actually support a UH command. By default, it starts in 12bit mode and if switched to 8bit mode with a UL command, there is no way "going back" via serial commands. Therefore, as a hack, we reset the Teensy via tycmd (bless you @Koromix for tytools!). This requires some refactoring to gracefully handle the port close: the serial input stream reading and command forwarding are "swapped": input stream is now a goroutine that is respawned each time the bitdepth changes, while the command forwarding is the "main" body. These changes are intended to be backwards compatible with older versions of Play (which send no bitdepth commands and assume that the driver sets 8bit mode by default). These changes also introduce several assumptions: - that the bitdepth command is always the first command to be sent by Play - that the Flex firmware is running at 12bit by default (seems to be the case with v4 and v5) Tested the changes using the following scenarios: - v4 mat with Play on `main` - OK - v4 mat with Play on `flex-v5` 8bit mode - OK - v4 mat with Play on `flex-v5` 12bit mode - works, but 12bit seems to be buggy with v4 firmware (unrelated to these changes) - v4 mat with Play on `flex-v5` switching bitdepth from UI - OK - v5 mat with Play on `main`- works, but 8bit mode usage is useless since Autozero Threshold does not work - v5 mat with Play on `flex-v5` 8bit mode - same as above - v5 mat with Play on `flex-v5` 12bit mode - OK - v5 mat with Play on `flex-v5` switching bitdepth from UI - OK Also USB plug-unplug autoreconnect tests: - v4 mat with Play on `main` - OK - v4 mat with Play on `flex-v5` 8bit - OK - v4 mat with Play on `flex-v5` 12bit - OK - v5 mat with Play on `main` - OK - v5 mat with Play on `flex-v5` 8bit - OK - v5 mat with Play on `flex-v5` 12bit - OK Remaining issues/questions: - the driver is no longer a single binary, because it requires `tycmd` to be present on the system. tycmd is available cross-platform, so in theory we just need to distribute it together?
yfyf
commented
Mar 21, 2025
|
|
||
| logger.Info("Closing port and waiting for reader to stop") | ||
| port.Close() | ||
| <-readerDoneChan |
There was a problem hiding this comment.
I'm not sure this is strictly necessary - once we close the port, the reader dies and that's it. But I think it's a good to get an ack here, so that we don't accidentally spawn multiple readers or anything like that, in case of future refactoring and whatnot?
yfyf
commented
Mar 21, 2025
| return | ||
| } | ||
|
|
||
| input, err := reader.ReadByte() | ||
| if err != nil { | ||
| logger.WithField("err", err).Error("Error reading from serial port") |
There was a problem hiding this comment.
When the port is closed on bitdepth change, this will produce a a single error log. I think this is preferable to silently dying though, because there might be other situations when reading fails that we might want to know about.
yfyf
commented
Mar 21, 2025
| // consecutive bytes (big-endian). | ||
| length_msb = input | ||
| state = HEADER_READ_LENGTH_LSB | ||
| case state == HEADER_READ_LENGTH_LSB: |
There was a problem hiding this comment.
Minor refactoring here: split the read length into two states for reading MSB and LSB to avoid having custom error handling/logging when reading the input. Could be left as is.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See commit messages for more details. They are to be squashed, but keeping them separate so you can see the initial "naive" implementation I did and then the horrendous hack I had to resort to.
If you want to try it with Play, use the flex-v5 branch