From b27433f9ee428ab1713f9efc0d3226f6743ab74e Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Mon, 29 Jun 2020 21:29:42 +0200 Subject: [PATCH] keyboardio/Atreus: Add some explanatory comments While the code became shorter with the previous commit, it did not become much easier to understand and follow. Some of the choices made weren't self explanatory. For that reason, lets put comments around the relevant parts, that explain why we've done it that way. Signed-off-by: Gergely Nagy --- .../device/keyboardio/Atreus2.cpp | 26 ++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/src/kaleidoscope/device/keyboardio/Atreus2.cpp b/src/kaleidoscope/device/keyboardio/Atreus2.cpp index 1bf38642..d5eb707f 100644 --- a/src/kaleidoscope/device/keyboardio/Atreus2.cpp +++ b/src/kaleidoscope/device/keyboardio/Atreus2.cpp @@ -1,6 +1,6 @@ /* -*- mode: c++ -*- * Keyboardio Atreus hardware support for Kaleidoscope - * Copyright (C) 2019 Keyboard.io, Inc + * Copyright (C) 2019, 2020 Keyboard.io, Inc * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -22,8 +22,11 @@ #include "kaleidoscope/Runtime.h" #include "kaleidoscope/driver/keyscanner/Base_Impl.h" +// We're using the `kaleidoscope::device::keyboardio` namespace, and set up the +// aliases here, so that they're in the global namespace, within the scope of +// this file. We do that, because of how templates are resolved and evaluated, +// see more just down below! using namespace kaleidoscope::device::keyboardio; - using KeyScannerProps = typename AtreusProps::KeyScannerProps; using KeyScanner = typename AtreusProps::KeyScanner; @@ -31,17 +34,34 @@ namespace kaleidoscope { namespace device { namespace keyboardio { +// `KeyScannerProps` here refers to the alias set up above. We do not need to +// prefix the `matrix_rows` and `matrix_columns` names within the array +// declaration, because those are resolved within the context of the class, so +// the `matrix_rows` in `KeyScannerProps::matrix_row_pins[matrix_rows]` gets +// resolved as `KeyScannerProps::matrix_rows`. const uint8_t KeyScannerProps::matrix_rows; const uint8_t KeyScannerProps::matrix_columns; - constexpr uint8_t KeyScannerProps::matrix_row_pins[matrix_rows]; constexpr uint8_t KeyScannerProps::matrix_col_pins[matrix_columns]; +// Resolving is a bit different in case of templates, however: the name of the +// array is resolved within the scope of the namespace and the class, but the +// array size is not - because it is a template. Therefore, we need a fully +// qualified name there - or an alias in the global scope, which we set up just +// above. template<> uint16_t KeyScanner::previousKeyState_[KeyScannerProps::matrix_rows] = {}; template<> uint16_t KeyScanner::keyState_[KeyScannerProps::matrix_rows] = {}; template<> uint16_t KeyScanner::masks_[KeyScannerProps::matrix_rows] = {}; template<> uint8_t KeyScanner::debounce_matrix_[KeyScannerProps::matrix_rows][KeyScannerProps::matrix_columns] = {}; +// We set up the TIMER1 interrupt vector here. Due to dependency reasons, this +// cannot be in a header-only driver, and must be placed here. +// +// Timer1 is responsible for setting a property on the KeyScanner, which will +// tell it to do a scan. We use this to make sure that scans happen at roughly +// the intervals we want. We do the scan outside of the interrupt scope for +// practical reasons: guarding every codepath against interrupts that can be +// reached from the scan is far too tedious, for very little gain. ISR(TIMER1_OVF_vect) { Runtime.device().keyScanner().do_scan_ = true; }