Fix Seika Notetaker buttons#12598
Conversation
See test results for failed build of commit 2ff7f421e0 |
|
@moyanming - would you like to test this PR build and see if the issue with buttons is still an issue? |
Sure, I will test the PR build right now. |
|
|
||
|
|
||
| def _getRoutingIndexes(routingKeys: bytes) -> Set[int]: | ||
| return {i * 8 + j for i in range(len(routingKeys)) for j in range(8) if routingKeys[i] & (1 << j)} |
There was a problem hiding this comment.
This one is a little harder, I suspect it could be simplified. There are likely only so many values for routingKeys, I'd suggest a brute force unit test that tests them all (eg that the new algorithm gives the same out put as this one). Once simplified the test can probably be deleted.
Probably something like:
| return {i * 8 + j for i in range(len(routingKeys)) for j in range(8) if routingKeys[i] & (1 << j)} | |
| routingKeyBitFlags = routingKeys | |
| bitValues = [1 << i for i in range(8)] | |
| routingIndexes = [] | |
| routingIndexTest = 0 | |
| for byte in routingKeyBitFlags: | |
| for bitValue in bitValues: | |
| if bitValue & byte: | |
| routingIdexes.append(routingIndexTest) | |
| routingIndexTest += 1 |
Or if you want to still do it in a list comprehension style
| return {i * 8 + j for i in range(len(routingKeys)) for j in range(8) if routingKeys[i] & (1 << j)} | |
| bitsPerByte = 8 | |
| bitValues = [1 << i for i in range(bitsPerByte)] | |
| return { | |
| bitIndex + (byteIndex * bitsPerByte) | |
| for bitIndex, bitValue in enumerate(bitValues) | |
| for byteIndex, bitFlags in enumerate(routingKeys) | |
| if bitValue & bitFlags |
There was a problem hiding this comment.
I took a different approach here, closer to the approach suggested above. Let me know what you think.
The current testing should provide enough coverage here I think, unless Nippon releases a seika notetaker device with a very large number of keys.
|
@moyanming - is it okay that we are publishing the pdf of this driver within our repository (and the transcribed version)? |
Hi, @seanbudd Regards |
Hi, @seanbudd |
037fc91 to
353b6b4
Compare
Link to issue number:
None
Summary of the issue:
A spec for the braille device, seika notetaker, was provided here. The driver was hard to maintain and understand. An issue was raised in that comment which noted that keys that did not exist and were not labelled were being checked for.
Description of how this pull request fixes the issue:
The driver and testing has been updated to match it using examples from the spec.
Testing strategy:
Unit tests have been added which follow the spec:
Known issues with pull request:
None
Change log entries:
Bug fixes
- Improvements to the Seika Notetaker driverCode Review Checklist: