Add Keyboard I/O peripheral with tests and example - #453
Conversation
| /* | ||
| * keyboardLedMatrix.c | ||
| * Reads characters from the keyboard peripheral and displays the most recent | ||
| * one in the center of the LED matrix using a 5x7 bitmap font. |
There was a problem hiding this comment.
It might be obvious, but i like to make things super obvious - can you write here that, as a prerequisite, users should instantiate a matrix and keyboard peripheral before running this program?
There was a problem hiding this comment.
I’ve updated the header comment to explicitly state the prerequisite.
It now mentions that, before running the program, the user must add both the LED Matrix and Keyboard peripherals to the Ripes I/O tab, using the default identifiers (LED_MATRIX_0_* and KEYBOARD_0_*).
| QMutexLocker lock(&m_bufMutex); | ||
| const int count = m_keyBuffer.size(); | ||
| const uint8_t ch = m_lastKey; | ||
| lock.unlock(); |
There was a problem hiding this comment.
Please use a scoped lock instead of a lock/unlock pattern.
There was a problem hiding this comment.
I replaced the manual lock.unlock() pattern with a scoped block, so the mutex is now released automatically when the block ends.
| const int key = event->key(); | ||
| uint8_t ascii = 0; | ||
|
|
||
| if (key >= Qt::Key_A && key <= Qt::Key_Z) | ||
| ascii = static_cast<uint8_t>('A' + (key - Qt::Key_A)); | ||
| else if (key >= Qt::Key_0 && key <= Qt::Key_9) | ||
| ascii = static_cast<uint8_t>('0' + (key - Qt::Key_0)); | ||
| else if (key == Qt::Key_Space) | ||
| ascii = static_cast<uint8_t>(' '); |
There was a problem hiding this comment.
huh - is there really no built-in Qt function for this?
There was a problem hiding this comment.
Qt provides QKeyEvent::text(), which gives the actual character generated by the key press while taking the current keyboard layout and modifiers into account. That turned out to be exactly what we needed here, so I replaced the manual Qt::Key_* to ASCII mapping with event->text().toLatin1().
| else if (key == Qt::Key_Space) | ||
| ascii = static_cast<uint8_t>(' '); | ||
|
|
||
| if (ascii != 0) { |
There was a problem hiding this comment.
Zero checks aren't the best. If you insist on using this pattern, do std::optional<uint8_t> ascii.
There was a problem hiding this comment.
Agreed. I removed the sentinel 0 check and switched to std::optional<uint8_t>, so the presence or absence of a decoded character is now represented explicitly.
| if (offset == 0) { | ||
| QMutexLocker lock(&m_bufMutex); | ||
| const uint8_t val = m_keyBuffer.isEmpty() ? 0 : m_keyBuffer.dequeue(); | ||
| lock.unlock(); | ||
| QMetaObject::invokeMethod( | ||
| this, [this]() { refreshStatusLabel(); }, Qt::QueuedConnection); | ||
| return val; | ||
| } | ||
| if (offset == 4) { | ||
| QMutexLocker lock(&m_bufMutex); | ||
| return static_cast<VInt>(m_keyBuffer.size()); | ||
| } | ||
| return 0; |
There was a problem hiding this comment.
- Although it's small, please add an enum for the register map, and do a switch statement here. Magic constants are bad.
- To keep the code a bit dry, i'd just lock the m_bufMutex at the start of this function. Again, use scoped locks instead of lock/unlocks.
There was a problem hiding this comment.
I added a RegMap enum for the register offsets, replaced the if chain with a switch over KEY_DATA and KEY_STATUS, and moved to a single scoped QMutexLocker at the start of the function to avoid both magic constants and repeated locking logic.
| } | ||
|
|
||
| void IOKeyboard::ioWrite(AInt offset, VInt value, unsigned) { | ||
| if (offset == 4 && value != 0) { |
There was a problem hiding this comment.
use the enum register map value that you are going to define.
There was a problem hiding this comment.
After introducing the RegMap enum, I updated the code to use RegMap::KEY_DATA and RegMap::KEY_STATUS everywhere instead of the raw offset values.
3 participants
Adds a memory-mapped Keyboard peripheral, available from the I/O panel
alongside the existing LED Matrix / Switches / D-Pad.
Behavior
The widget captures key presses while focused (letters, digits, space) and
also exposes on-screen buttons. Captured ASCII codes are pushed into an
internal FIFO whose depth is configurable via the
Buffer sizeparameter(default 16). Auto-repeat is suppressed.
Memory map
Example
examples/C/keyboardLedMatrix.c— pollsKEY_STATUS, readsKEY_DATA,and renders the character on the LED Matrix using a 5x7 font.
Tests
test/tst_io_keyboard.cpp— covers FIFO ordering, STATUS/DATAconsistency, empty-buffer reads, and reset.