Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
DurvalMenezes opened this issue Nov 6, 2024 · 3 comments

Comments

@DurvalMenezes
Copy link

DurvalMenezes commented Nov 6, 2024

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;

  1. Compile a new hex for that light, continuing the eg above:
$ ./make hw/wurkkos/ts10/rgbaux
  1. flash the resulting hex to the flashlight:
sudo avrdude -p avr32dd20 -c serialupdi -P /dev/ttyUSB0 -Uflash:w:hex/anduril.wurkkos-ts10-rgbaux.hex
  1. 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!

@DurvalMenezes DurvalMenezes changed the title Disabling Simple UI at compile time (#undef USE_SIMPLE_UI) also disables all strobe mode Disabling Simple UI at compile time (#undef USE_SIMPLE_UI) also disables all strobe modes Nov 6, 2024
@DurvalMenezes DurvalMenezes changed the title Disabling Simple UI at compile time (#undef USE_SIMPLE_UI) also disables all strobe modes BUG: Disabling Simple UI at compile time (#undef USE_SIMPLE_UI) also disables all strobe modes Nov 6, 2024
@SammysHP
Copy link
Contributor

SammysHP commented Nov 6, 2024

Caused by 4f7f90d.

Duplicate of #94.

Fixed with #97.

@DurvalMenezes
Copy link
Author

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
Copy link
Contributor

SammysHP commented Nov 6, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants