fixes primary keymap cycling

sdcard/snippets/multibow.lua

@@ -31,8 +31,15 @@ require(mb.path .. "routehandlers")
 
 
 mb.brightness = 0.4
+-- The registered keymaps, indexed by their names (.name field).
 mb.keymaps = {}
+-- The ordered sequence of primary keymap, in the sequence they were
+-- registered.
+mb.primary_keymaps = {}
+-- The currently activated keymap.
 mb.current_keymap = nil
+
+-- A temporary keymap while grabbing.
 mb.grab_keymap = nil
 
 
@@ -49,7 +56,6 @@ function mb.tap(keyno, key, ...)
   end
 end
 
-
 -- Registers a keymap (by name), so it can be easily activated later by its name.
 -- Multiple keymaps can be registered. Keymaps can be either "primary" by
 -- default, or permanent or secondary keymaps.
@@ -73,13 +79,14 @@ function mb.register_keymap(keymap)
   -- register
   mb.keymaps[name] = keymap
   -- ensure that first registered keymap also automatically gets activated
-  -- (albeit the LEDs will only update later).
+  -- (albeit the LEDs will only update later). Also maintain the (ordered)
+  -- sequence of registered primary keymaps.
   if not (keymap.permanent or keymap.secondary) then
     mb.current_keymap = mb.current_keymap or keymap
+    table.insert(mb.primary_keymaps, keymap)
   end
 end
 
-
 -- Returns the list of currently registered keymaps; this list is a table,
 -- with its registered keymaps at indices 1, 2, ...
 function mb.registered_keymaps()
@@ -90,35 +97,57 @@ function mb.registered_keymaps()
   return keymaps
 end
 
+-- Returns the list of currently registered primary keymaps, in the same order
+-- as they were registered. First primary is at index 1, second at 2, ...
+function mb.registered_primary_keymaps()
+  return mb.primary_keymaps
+end
 
 -- Cycles through the available (primary) keymaps, ignoring secondary and
 -- permanent keymaps. This is convenient for assigning primary keymap switching
 -- using a key on the Keybow device itself.
 function mb.cycle_primary_keymaps()
-  local first_name
-  local next = false
-  local next_name
-  for name, keymap in pairs(mb.keymaps) do
-    if not (keymap.permanent or keymap.secondary) then
-      -- Remembers the first "primary" keymap.
-      first_name = first_name or name
-      -- Notice if we just pass the current keymap and then make sure to pick
-      -- up the following primary keymap.
-      if keymap == mb.current_keymap then
-        next = true
-      elseif next then
-        next_name = name
-        next = false
+  local km = mb.current_keymap
+  if km == nil then return end
+  -- If this is a secondary keymap, locate its corresponding primary keymap.
+  if km.secondary then
+    if not km.shift_to then
+      -- No SHIFT's shift_to cyclic chain available, so rely on the naming
+      -- schema instead and try to locate the primary keymap with the first
+      -- match instead. This assumes that the name of the secondary keymaps
+      -- have some suffix and thus are longer than the name of their
+      -- corresponding primary keymap.
+      for idx, pkm in ipairs(mb.primary_keymaps) do
+        if string.sub(km.name, 1, #pkm.name) == pkm.name then
+          km = pkm
+          break
+        end
       end
+      -- Checks if locating the primary keymap failed and then bails out
+      -- immediately.
+      if km.secondary then return end
+    else
+      -- Follows the cyclic chain of SHIFT's shift_to keymaps, until we get
+      -- to the primary keymap in the cycle, or until we have completed one
+      -- cycle.
+      repeat
+        km = km.shift_to
+        if not km or km == mb.current_keymap then
+          return
+        end
+      until not(km.secondary)
     end
   end
-  next_name = next_name or first_name
-  if first_name then
-    mb.activate_keymap(next_name)
+  -- Move on to the next primary keymap, rolling over at the end of our list.
+  for idx, pkm in ipairs(mb.primary_keymaps) do
+    if pkm == km then
+      idx = idx + 1
+      if idx > #mb.primary_keymaps then idx = 1 end
+      mb.activate_keymap(mb.primary_keymaps[idx].name)
+    end
   end
 end
 
-
 -- Activates a specific keymap by name. Please note that it isn't necessary
 -- to "activate" permanent keymaps at all (and thus this deed cannot be done).
 function mb.activate_keymap(name)
@@ -150,7 +179,6 @@ function mb.set_brightness(brightness)
   mb.activate_leds()
 end
 
-
 -- Sets key LED to specific color, taking brightness into consideration.
 -- The color is a triple (table) with the elements r, g, and b. Each color
 -- component is in the range [0..1].
@@ -163,7 +191,6 @@ function mb.led(keyno, color)
   end
 end
 
-
 -- Restores Keybow LEDs according to current keymap and the permanent keymaps.
 function mb.activate_leds()
   keybow.clear_lights()
@@ -185,7 +212,6 @@ function mb.activate_leds()
   end
 end
 
-
 -- Helper function that iterates over all keymap elements but skipping non-key
 -- bindings.
 function mb.activate_keymap_leds(keymap)
@@ -197,7 +223,6 @@ function mb.activate_keymap_leds(keymap)
   end
 end
 
-
 -- Disables the automatic Keybow lightshow and sets the key LED colors. This
 -- is a well-known (hook) function that gets called by the Keybow firmware
 -- after initialization immediately before waiting for key events.
@@ -209,4 +234,5 @@ function setup()
   mb.activate_leds()
 end
 
+
 return mb -- module

spec/snippets/multibow_spec.lua

@@ -33,69 +33,115 @@ describe("multibow", function()
         mb = require("snippets/multibow")
     end)
 
-    insl(function()
-        it("adds permanent keyboard layout, but doesn't activate it", function()
-            local permkm = {
-                name="permanent",
-                permanent=true
-            }
-            assert.is_nil(mb.keymaps["permanent"])
-            mb.register_keymap(permkm)
-            assert.is.equal(mb.keymaps["permanent"], permkm)
-            assert.is_nil(mb.current_keymap)
-        end)
+    inslit("adds permanent keyboard layout, but doesn't activate it", function()
+        local permkm = {
+            name="permanent",
+            permanent=true
+        }
+        assert.is_nil(mb.keymaps["permanent"])
+        mb.register_keymap(permkm)
+        assert.is.equal(mb.keymaps["permanent"], permkm)
+        assert.is_nil(mb.current_keymap)
     end)
 
-    insl(function()
-        it("checks multibow module is fresh again", function()
-            assert.is_nil(mb.keymaps["permanent"])
-        end)
+    inslit("checks multibow module is fresh again", function()
+        assert.is_nil(mb.keymaps["permanent"])
     end)
 
-    insl(function()
-        it("adds permanent, then two primary layouts, activates only first primary layout", function()
-            local permkm = {
-                name="permanent",
-                permanent=true
-            }
-            mb.register_keymap(permkm)
-            local prim1km = { name="bavaria-one" }
-            local prim2km = { name="primary-two" }
-            mb.register_keymap(prim1km)
-            mb.register_keymap(prim2km)
-            assert.is.equal(mb.current_keymap, prim1km)
-        end)
+    inslit("adds permanent, then two primary layouts, activates only first primary layout", function()
+        local permkm = {
+            name="permanent",
+            permanent=true
+        }
+        mb.register_keymap(permkm)
+        local prim1km = { name="bavaria-one" }
+        local prim2km = { name="primary-two" }
+        mb.register_keymap(prim1km)
+        mb.register_keymap(prim2km)
+        assert.is.equal(prim1km, mb.current_keymap)
     end)
 
-    insl(function()
-        it("adds secondary, then primary layout, activates only primary layout", function()
-            local primkm = { name="berlin" }
-            local seckm = { name="munich", secondary=true }
-            mb.register_keymap(seckm)
-            mb.register_keymap(primkm)
-            assert.is.equal(mb.current_keymap, primkm)
-        end)
+    inslit("sequence of primary keymaps is in registration order", function()
+        local prim1km = { name="last" }
+        local prim2km = { name="first" }
+        mb.register_keymap(prim1km)
+        mb.register_keymap(prim2km)
+        assert.is.same(mb.registered_primary_keymaps(), {prim1km, prim2km})
     end)
 
-    insl(function()
-        it("sets up multibow, activates lights", function()
-            local s = spy.on(_G, "setup")
-            local al = spy.on(mb, "activate_leds")
-            
-            _G.setup()
-            assert.spy(s).was.called(1)
-            assert.spy(al).was.called(1)
-
-            s:revert()
-            al:revert()
-        end)
+    inslit("adds secondary, then primary layout, activates only primary layout", function()
+        local primkm = { name="berlin" }
+        local seckm = { name="munich", secondary=true }
+        mb.register_keymap(seckm)
+        mb.register_keymap(primkm)
+        assert.is.equal(primkm, mb.current_keymap)
     end)
 
-    insl(function()
-        it("has more keys", function()
-            assert.is_not_nil(keybow.F13)
-            assert.is.equal(keybow.F13, 0x68)
-        end)
+    inslit("cycles primary keymaps based on primary-secondary names substring match", function()
+        -- on purpose, the names of the primary keymaps are in reverse lexical order,
+        -- to make sure that cycling follows the registration order, but not the
+        -- name order.
+        local prim1km = { name= "last" }
+        local sec1km = { name="last-shift", secondary=true }
+        local sec2km = { name="xlast-shift", secondary=true}
+        local prim2km = { name= "first" }
+        mb.register_keymap(prim1km)
+        mb.register_keymap(prim2km)
+        mb.register_keymap(sec1km)
+        mb.register_keymap(sec2km)
+        assert.is.equal(4, #mb.registered_keymaps())
+        assert.is.same(mb.registered_primary_keymaps(), {prim1km, prim2km})
+
+        -- cycles from secondary to next primary
+        mb.activate_keymap(sec1km.name)
+        mb.cycle_primary_keymaps()
+        assert.is.equal(prim2km.name, mb.current_keymap.name)
+        -- cycles from last primary to first primary
+        mb.cycle_primary_keymaps()
+        assert.is.equal(prim1km.name, mb.current_keymap.name)
+        -- cannot cycle from misnamed secondary without shift_to
+        mb.activate_keymap(sec2km.name)
+        mb.cycle_primary_keymaps()
+        assert.is.equal(sec2km.name, mb.current_keymap.name)
+    end)
+
+    inslit("cycles primary keymaps based on shift_to", function()
+        -- on purpose, the names of the primary keymaps are in reverse lexical order,
+        -- to make sure that cycling follows the registration order, but not the
+        -- name order.
+        local prim1km = { name= "last" }
+        local sec1km = { name="last-shift", secondary=true, shift_to }
+        prim1km.shift_to = sec1km
+        local prim2km = { name= "first" }
+        mb.register_keymap(prim1km)
+        mb.register_keymap(prim2km)
+        mb.register_keymap(sec1km)
+        assert.is.same(mb.registered_primary_keymaps(), {prim1km, prim2km})
+
+        -- cycles from secondary to next primary
+        mb.activate_keymap(sec1km.name)
+        mb.cycle_primary_keymaps()
+        assert.is.equal(prim2km.name, mb.current_keymap.name)
+        -- cycles from last primary to first primary
+        mb.cycle_primary_keymaps()
+        assert.is.equal(prim1km.name, mb.current_keymap.name)
+    end)
+
+    inslit("sets up multibow, activates lights", function()
+        local s = spy.on(_G, "setup")
+        local al = spy.on(mb, "activate_leds")
+        
+        _G.setup()
+        assert.spy(s).was.called(1)
+        assert.spy(al).was.called(1)
+
+        s:revert()
+        al:revert()
+    end)
+
+    inslit("has more keys", function()
+        assert.is_not_nil(keybow.F13)
+        assert.is.equal(0x68, keybow.F13)
     end)
 
 end)