Skip to content
Open
Show file tree
Hide file tree
Changes from 18 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,20 +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::overlays_base_;
Overlay *ColormapOverlay::overlays_;
uint8_t ColormapOverlay::overlay_count_;

void ColormapOverlay::setup() {
::LEDPaletteTheme.reservePalette();

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

::EEPROMSettings.seal();

if (!::EEPROMSettings.isValid()) {
// When settings are invalid, better to clean it all up to prevent unwanted things from happening
::EEPROMSettings.invalidate();
Comment on lines +52 to +53
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Re: #1434 (comment)

Is this the correct way to handle this?

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 believe so. the seal is taken care of automatically. and isValid is a whole-system level thing.
This should be the right pattern

EventHandlerResult Keyclick::onSetup() {
  // Request storage space and load settings
  bool success = ::EEPROMSettings.requestSliceAndLoadData(&settings_base_, &settings_);

  // If settings weren't loaded successfully, use defaults
  if (!success) {
    settings_.enabled = true;
    settings_.version = 0x01;
    updateSettings();
  }

return;
}

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

bool ColormapOverlay::hasOverlay(KeyAddr k) {
uint8_t top_layer = Layer.mostRecent();
uint8_t layer_index = Layer.lookupActiveLayer(k);

bool found_match_on_lower_layer = false;
for (uint8_t i{0}; i < overlay_count_; ++i) {
for (uint8_t i{0}; i < overlay_count_; i++) {
Overlay overlay = overlays_[i];
if (overlay.addr == k) {
if ((overlay.layer == top_layer) || (overlay.layer == layer_wildcard)) {
Expand Down Expand Up @@ -82,6 +101,69 @@ 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
// -> No overload for signed integers
// Ref: src/kaleidoscope/device/Base.h
// -> 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_++;

if (overlay_count_ >= max_overlays_) {
// TODO(anyone): For now we just break the loop. Once focus implements
// support for reporting back errors, this should report why we break
// the loop
Comment on lines +151 to +153
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Left a TODO to follow up on this discussion: #1434 (comment)

break;
}
}

i++;
}
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 @@ -60,6 +61,7 @@ class ColormapOverlay : public kaleidoscope::Plugin {
// as a separate parameter.
template<uint8_t _overlay_count>
void configureOverlays(Overlay const (&overlays)[_overlay_count]) {
static_assert(_overlay_count <= max_overlays_);
// Delete old overlays if they exist
if (overlays_ != nullptr) {
delete[] overlays_;
Expand All @@ -79,12 +81,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 +115,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 @@ -127,8 +131,10 @@ class ColormapOverlay : public kaleidoscope::Plugin {
}

private:
Overlay *overlays_;
uint8_t overlay_count_;
static uint16_t overlays_base_;
static const uint8_t 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.

Following up on #1434 (comment):

layer_count is far too few, as an overlay only applies to a single key. 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.

Copy link
Member

Choose a reason for hiding this comment

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

Understood. I think that starting at a fixed # like 64 seems fine. if we find folks needing it increased, we can do that.

static Overlay *overlays_;
static uint8_t overlay_count_;
cRGB selectedColor;

bool hasOverlay(KeyAddr k);
Expand Down