ToyKeeper / anduril

Anduril 2 Flashlight Firmware and FSM UI Toolkit
GNU General Public License v3.0
240 stars 60 forks source link

BUG: Disabling Simple UI at compile time (#undef USE_SIMPLE_UI) also disables all strobe modes #116

Open DurvalMenezes opened 6 days ago

DurvalMenezes commented 6 days ago

As per the title.

This does not happen with r2024-04-20, but does happen with current trunk (e43203f).

How to reproduce: in current trunk (e43203f):

  1. Add

    #undef USE_SIMPLE MODE

    eg, to the end of hw/wurkkos/ts10/rgbaux/anduril.h;

  2. Compile a new hex for that light, continuing the eg above:

    $ ./make hw/wurkkos/ts10/rgbaux
  3. flash the resulting hex to the flashlight:

    sudo avrdude -p avr32dd20 -c serialupdi -P /dev/ttyUSB0 -Uflash:w:hex/anduril.wurkkos-ts10-rgbaux.hex
  4. From off, press 3H and, instead of the expected strobe (by default, candle mode), absolutely nothing happens: the flashlight remains in the 'off' state.

I've examined the code and I'm pretty sure the problem is caused in anduril/ui/off-mode.c by the 3H handling code being incorrectly placed inside a #ifdef USE_SIMPLE_MODE // #endif block; this does fix it for me:

--- a/ui/anduril/off-mode.c
+++ b/ui/anduril/off-mode.c
@@ -316,38 +316,38 @@ uint8_t off_state(Event event, uint16_t arg) {

     #ifdef USE_SIMPLE_UI
     #ifdef USE_EXTENDED_SIMPLE_UI
     if (cfg.simple_ui_active) {
         return EVENT_NOT_HANDLED;
     }
     #endif  // ifdef USE_EXTENDED_SIMPLE_UI

+    // 10 clicks: enable simple UI
+    else if (event == EV_10clicks) {
+        blink_once();
+        cfg.simple_ui_active = 1;
+        save_config();
+        return EVENT_HANDLED;
+    }
+    #endif  // ifdef USE_SIMPLE_UI
+
     // click, click, long-click: strobe mode
     #ifdef USE_STROBE_STATE
     else if (event == EV_click3_hold) {
         set_state(strobe_state, 0);
         return EVENT_HANDLED;
     }
     #elif defined(USE_BORING_STROBE_STATE)
     else if (event == EV_click3_hold) {
         set_state(boring_strobe_state, 0);
         return EVENT_HANDLED;
     }
     #endif

-    // 10 clicks: enable simple UI
-    else if (event == EV_10clicks) {
-        blink_once();
-        cfg.simple_ui_active = 1;
-        save_config();
-        return EVENT_HANDLED;
-    }
-    #endif  // ifdef USE_SIMPLE_UI
-
     #ifdef USE_MOMENTARY_MODE
     // 5 clicks: momentary mode
     else if (event == EV_5clicks) {
         blink_once();
         set_state(momentary_state, 0);
         return EVENT_HANDLED;
     }
     #endif

I'd like for someone else to confirm the bug and my analysis above, and then I will submit a proper PR.

TIA!

SammysHP commented 6 days ago

Caused by 4f7f90df760f1bd215ce1b2ec671d62b332690c6.

Duplicate of #94.

Fixed with #97.

DurvalMenezes commented 6 days ago

Thanks @SammysHP. I reviewed the open issues before opening this one, but missed #94.

Is there a fork or something with these bug-fixing PRs applied, or at least an easier way to find them without having to go through each one? It would IMO help everyone to avoid 'reinventing the wheel' as I just did.

SammysHP commented 6 days ago

No, eventually they will be merged upstream. You should maintain your own fork with the patches you need.