Skip to content
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@
* You should have received a copy of the GNU General Public License along with
* this program. If not, see <http://www.gnu.org/licenses/>.
*/

#include <Kaleidoscope.h>
#include <Kaleidoscope-LEDControl.h>
#include <Kaleidoscope-EEPROM-Settings.h>
#include <Kaleidoscope-LED-Palette-Theme.h>
#include <Kaleidoscope-Colormap-Overlay.h>

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"cpu": {
"fqbn": "keyboardio:gd32:keyboardio_model_100",
"port": ""
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
default_fqbn: keyboardio:gd32:keyboardio_model_100
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,39 @@
#include "kaleidoscope/key_defs.h" // for Key, KEY_FLAGS, Key_NoKey, LockLayer
#include "kaleidoscope/layers.h" // for Layer, Layer_
#include "kaleidoscope/plugin/LEDControl.h" // for LEDControl
#include <Kaleidoscope.h> // for Kaleidoscope
#include <Kaleidoscope-FocusSerial.h> // for Focus
#include <Kaleidoscope-LED-Palette-Theme.h> // for LEDPaletteTheme
#include <Kaleidoscope-EEPROM-Settings.h> // for EEPROMSettings

namespace kaleidoscope {
namespace plugin {
uint16_t ColormapOverlay::map_base_;
uint16_t ColormapOverlay::overlays_base_;
uint8_t ColormapOverlay::max_overlays_ = 64; // TODO(EvyBongers): figure this out. How determine a good maximum?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@obra do you have a way to determine a good value here, or is this pure guess work?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we want to be setting it to layer_count? that might (or might not) need an accessor.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

layer_count is far too few, as an overlay only applies to a single key. Of course we could set it to layer_count * Runtime.device().numKeys(), but if anyone is gonna need that number of overlays, they're better off using Colormap rather than ColormapOverlay.

Overlay *ColormapOverlay::overlays_;
uint8_t ColormapOverlay::overlay_count_;

void ColormapOverlay::setup() {
// It appears that a call to ::LEDPaletteTheme.reserveThemes() is needed
// because it's where palette_base_ gets initialized. Since this plugin (and
// possibly others) don't actually use themes, requesting memory for storing
// themes doesn't make much sense. Maybe initialisation of palette_base_
// could be moved to a setup() method, though maybe the palette should be
// split from palette theme altogether?
map_base_ = ::LEDPaletteTheme.reserveThemes(1);

overlays_base_ = ::EEPROMSettings.requestSlice(max_overlays_ * sizeof(Overlay));

::EEPROMSettings.seal();

if (!::EEPROMSettings.isValid()) {
// TODO(EvyBongers): figure this out. What to do when the settings are out of sync...

return;
}

Kaleidoscope.storage().get(overlays_base_, overlays_);
}

bool ColormapOverlay::hasOverlay(KeyAddr k) {
Expand Down Expand Up @@ -84,6 +109,60 @@ EventHandlerResult ColormapOverlay::beforeSyncingLeds() {
return EventHandlerResult::OK;
}

EventHandlerResult ColormapOverlay::onFocusEvent(const char *input) {
if (!Runtime.has_leds)
return EventHandlerResult::OK;

const char *cmd = PSTR("colormap.overlay");

if (::Focus.inputMatchesHelp(input))
return ::Focus.printHelp(cmd);

if (!::Focus.inputMatchesCommand(input, cmd))
return EventHandlerResult::OK;

if (::Focus.isEOL()) {
for (uint8_t layer = 0; layer < layer_count; layer++) {
for (uint8_t key_index_ = 0; key_index_ < Runtime.device().numKeys(); key_index_++) {
KeyAddr k = KeyAddr(key_index_);
for (uint8_t overlay_index{0}; overlay_index < overlay_count_; ++overlay_index) {
Overlay overlay = overlays_[overlay_index];
if ((overlay.addr == k) && (overlay.layer == layer)) {
::Focus.send(overlay.palette_index);
}
}
::Focus.send(-1);
}
}
return EventHandlerResult::EVENT_CONSUMED;
}

overlays_ = nullptr;
overlay_count_ = 0;
uint16_t i = 0;
while (!::Focus.isEOL() && (i < (uint16_t)Runtime.device().numKeys() * layer_count)) {
int8_t color_index_;

// Ref: plugins/Kaleidoscope-FocusSerial/src/kaleidoscope/plugin/FocusSerial.h:99-115
// -> No overload for signed integers
// Ref: src/kaleidoscope/device/Base.h:90-92
// -> parseInt() seems to support signed values?
::Focus.read(color_index_);
if (color_index_ >= 0) {
uint8_t key_index_ = i % Runtime.device().numKeys();
uint8_t layer_ = (i - key_index_) / Runtime.device().numKeys();

overlays_[overlay_count_] = Overlay(layer_, KeyAddr(key_index_), color_index_);
overlay_count_++; // TODO(EvyBongers): how to make sure that we don't exceed max_overlays_
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any way to communicate over focus that an error occurred? I could just break the loop when the limit is reached, but I wonder if there are ways to report back why it happens. Of course, if the limit above were to be set to max_layers * numKeys this wouldn't happen, but that doesn't make any sense because if you're gonna set led colors on every key you should be using Colormap instead of Colormap-Overlay

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there isn't currently. There should be.

}
}
Runtime.storage().commit();

::LEDControl.refreshAll();

return EventHandlerResult::EVENT_CONSUMED;
}

} // namespace plugin
} // namespace kaleidoscope

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@

#pragma once

#include <stdint.h> // for uint8_t
#include <Arduino.h> // for PROGMEM
#include <stdint.h> // for uint8_t

#include "kaleidoscope/KeyAddr.h" // for KeyAddr
#include "kaleidoscope/device/device.h" // for cRGB
Expand All @@ -35,7 +36,7 @@
namespace kaleidoscope {
namespace plugin {

// Data structure for an individual qukey
// Data structure for an individual color overlay
struct Overlay {
uint8_t layer;
KeyAddr addr;
Expand All @@ -58,7 +59,7 @@ class ColormapOverlay : public kaleidoscope::Plugin {
// takes as its sole argument an array reference of size `_overlay_count`, so
// there's no need to use `sizeof` to calculate the correct size, and pass it
// as a separate parameter.
template<uint8_t _overlay_count>
template<uint8_t _overlay_count> // TODO(EvyBongers): how to make sure that we don't exceed max_overlays_
void configureOverlays(Overlay const (&overlays)[_overlay_count]) {
// Delete old overlays if they exist
if (overlays_ != nullptr) {
Expand All @@ -79,12 +80,12 @@ class ColormapOverlay : public kaleidoscope::Plugin {
void configureOverlays(uint8_t **overlays) {
// First count how many overlays we'll need
uint8_t count = 0;
for (int layer_ = 0; layer_ < _layer_count; layer_++) {
for (int key_index_ = 0; key_index_ < kaleidoscope_internal::device.matrix_rows * kaleidoscope_internal::device.matrix_columns; key_index_++) {
for (uint8_t layer_ = 0; layer_ < _layer_count; layer_++) {
for (uint8_t key_index_ = 0; key_index_ < kaleidoscope_internal::device.matrix_rows * kaleidoscope_internal::device.matrix_columns; key_index_++) {
int8_t color_index_ = overlays[layer_][key_index_];
if (color_index_ >= 0 && color_index_ < ::LEDPaletteTheme.getPaletteSize() &&
color_index_ != no_color_overlay) {
count++;
count++; // TODO(EvyBongers): how to make sure that we don't exceed max_overlays_
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per your suggestion, I added a static_assert in the method above, but since I need to count the number if overlays here, it seems impossible to use that here. Any ideas on how I could validate that here? Or should I move the logic for finding all specified overlays to the macro below?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have an obvious answer for you here. What happens if you just return when they've gone over the limit?

}
}
}
Expand Down Expand Up @@ -113,12 +114,14 @@ class ColormapOverlay : public kaleidoscope::Plugin {
overlays_ = new_overlays;
overlay_count_ = count;
}

// A wildcard value for an overlay that applies on every layer.
static constexpr int8_t layer_wildcard{-1};
static constexpr int8_t no_color_overlay{-1};

EventHandlerResult onSetup();
EventHandlerResult beforeSyncingLeds();
EventHandlerResult onFocusEvent(const char *input);

~ColormapOverlay() {
if (overlays_ != nullptr) {
Expand All @@ -128,8 +131,10 @@ class ColormapOverlay : public kaleidoscope::Plugin {

private:
static uint16_t map_base_;
Overlay *overlays_;
uint8_t overlay_count_;
static uint16_t overlays_base_;
static uint8_t max_overlays_;
static Overlay *overlays_;
static uint8_t overlay_count_;
cRGB selectedColor;

bool hasOverlay(KeyAddr k);
Expand Down