ToyKeeper / anduril

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

D3AA emitter specific ramping issue (SST-20 deep red) #99

Open jan-Sanku opened 2 months ago

jan-Sanku commented 2 months ago

one of my 2 D3AAs behaves incorrectly when switching between smooth and stepped ramp at floor levels. instead of staying the same if the new floor is lower, or getting brighter if the new floor is higher, it always switches to/stays at the lower floor level.

tested on 0161-2024-04-20 with SST-20 deep red, could not reproduce with 591a on the same version. only other difference between the lights is that the red one also has red button LEDs, while the 519a has an RGB button. the misbehaving light is a fresh anduril install, and i factory reset it too.

steps to reproduce:

  1. go to the bottom of smooth ramp (set to level 1)
  2. 3C to switch to stepped ramp (floor is 10)
    • the light should now be go to level 10, but it stays at level 1
  3. ramp up (it correctly jumps to the 2nd step, 30 by default)
  4. ramp back down (it goes down to 10, as expected)
  5. 3C to switch back to smooth ramp
    • the light should stay at the stepped floor and allow ramping down, but it switches to smooth floor instead

not sure how it's possible that this is only happening with my SST-20 light. does it use a different driver or something?

jan-Sanku commented 2 months ago

i've been doing further testing and this is getting weird.

a new thing i've discovered with this particular light, still not the other d3aa, is that 1H from off to floor sometimes just doesn't turn on the emitters at all. if you keep holding it eventually fires up (at a much higher brightness than it should, as if it were on and ramping all along), and at that point you can ramp down too. this feels like something that you'd normally fix by jump starting, so i tried to enable jump start via 9H from off.

not only did i find out that the d3aa doesn't have jump starting, i found that 9H from off brings up the channel mode config that should be ON -> 9H! i'm not sure why this menu even exists on the single channel d3aa, that may need its own issue, but it definitely shouldn't be accessible from off.

either way, my sst-20 DR d3aa behaves very oddly at low levels, that much is sure. maybe there needs to be a fix for this emitter, or just allow jump starting on this light.

i triple checked that i'm running the correct model, 0161

has anyone else experienced similar issues?

dirtydancing commented 2 months ago

Channel mode config: for the D3AA, it is expected that channel mode config comes up via 9H from On and Off (and also from Lockout, actually). 9H from Off brings up channel mode config if there is no misc config. Apparently, the D3AA has no jump start level, at least there is no mention of a jump start level in its anduril.h, so there is no misc config for the D3AA (obviously there is also no tint ramping style, as the D3AA is a single-channel light).

This is stated in the text manual, which refers to "any mode", not just to On/Ramp for 9H for channel mode config. Actually, I have only recently added this to my diagram, if you are interested in a visual representation.

jan-Sanku commented 2 months ago

ah, i see the line you're referring to. it does conflict with at least one line that i've read, that's a little confusing. still, i don't really think the d3aa has a need for this menu to begin with, but perhaps that's a discussion for a separate issue.

Apparently, the D3AA has no jump start level, at least there is no mention of a jump start level in its anduril.h

yeah. i'm guessing the reason for that is that everyone was confident this new driver wouldn't need it (and it doesn't for my 519a!), but it seems like it might be helpful for the (red) sst-20

SiteRelEnby commented 2 months ago

The D3AA needs channel mode config for the aux channels.

I agree that perhaps the solution is a different firmware for SST20DR where jumpstart is enabled.

As for why the globals config menu is missing on the D3AA, it looks like by default it's only enabled when jumpstart is - see https://github.com/ToyKeeper/anduril/blob/trunk/ui/anduril/ramp-mode-fsm.h#L34-L37 - that part is at least easy to fix.

dirtydancing commented 2 months ago

ah, i see the line you're referring to. it does conflict with at least one line that i've read, that's a little confusing.

Agreed that the (while on) part is confusing, and seems to contradict these two lines above. I figure the on found its way into the text because 9H from On seems to be the straightforward way to enter the channel mode config menu (for example, I am not configuring the channel modes via 9H from Lockout. but with every light I have checked so far, this would have been possible).

Anyway, in case of doubt, I would recommend to consult the UI Reference Table at the end of the manual. For example, recently this (somewhat peculiar) question came up at my end, and also for this, the point of reference was the UI Reference Table.

still, i don't really think the d3aa has a need for this menu to begin with, but perhaps that's a discussion for a separate issue.

Agreed that this would be a separate issue. Currently, for single-channel lights with aux LEDs, the aux LED channels are channel modes for the 9H channel mode config menu. So this discussion would not be limited to the D3AA at all.

SiteRelEnby commented 2 months ago

Well, I have a D3AA with SST20DR and tested and mine doesn't show the same behaviour as @jan-Sanku 's, mine comes on at moon fine even with floor at 1 (unless this is an issue that only happens on 1.5V maybe?), but I created a firmware variant for it anyway.

Also fixed the bug with channel mode config being reachable from off on lights without globals config.

https://github.com/ToyKeeper/anduril/pull/100

SiteRelEnby commented 2 months ago

Firmware for anyone who wants it D3AA with jumpstart: anduril.hank-emisar-d3aa-jumpstart.hex.zip

Normal D3AA with globals config menu bug fixed: anduril.hank-emisar-d3aa.hex.zip

@jan-Sanku could you tell me if the default jumpstart level I picked is appropriate, or if it works with a lower setting?

dirtydancing commented 2 months ago

Also fixed the bug with channel mode config being reachable from off on lights without globals config.

Question for clarification: so this is a bug? In this case, I actually thought this is a feature. :-)

But I can see that this could also be a bug. So now, 9H from Off will go to the misc/globals config menu, even when there is no misc/globals menu point to configure (so just have no effect)? Might this not be even more confusing, given what the manual is saying, as cited above ("global button mappings which work at all times unless they're overridden", "any mode")? Thoughts?

jan-Sanku commented 2 months ago

@jan-Sanku could you tell me if the default jumpstart level I picked is appropriate, or if it works with a lower setting?

reflashed just now, will report back with my findings!

(of note: if the light was on for more than a few seconds, this doesn't seem to happen. only when it's "cold", so to speak)

SiteRelEnby commented 2 months ago

@dirtydancing yeah, it's a bug. channel_mode_state exists under all other states, and button events are handled in a "fall through" way, so if a state stacked on top of another one doesn't handle a specific event (e.g. 9H) and a lower state can, the lower state gets the event. So to block a specific combo from a lower state for one state we can add a blank handler to override the lower one, but if we want to block an event from all states except a specific one, we add a check to the lower state to make sure that the current state matches a defined list of allowed states it will respond in.

Example for restricting to one state: https://github.com/ToyKeeper/anduril/pull/100/files#diff-43f96dbac58ea64084b4c7644ab4d98a444fe519471af31ba663041eda7165d3R134

Example for blocking for a specific state only (blocking 3C to change channel when in tactical mode): https://github.com/ToyKeeper/anduril/blob/e43203f814c7040c49dbb7d4804e9f2008633b7b/ui/anduril/tactical-mode.c#L71-L74

jan-Sanku commented 2 months ago

so far, a jump start as low as lvl 5 has worked out for me! the emitters take maybe 0.25 seconds to actually turn on, but they do turn on.

i made sure to re-test, and indeed a jump start of 0 sometimes leaves them off indefinitely until i ramp up. curiously, this happens more rarely than it did in my earlier tests? but it does still happen (even got it on video this time!). but again, this is fixed by jump starting!

https://github.com/user-attachments/assets/cf310c33-9007-4499-9f26-336313f533d1

also, i was not able to reproduce the behavior that initially led me to open this issue, ie switching ramp modes causing the light to be at the wrong floor level. i'm not sure why, maybe reflashing fixed it?

SiteRelEnby commented 2 months ago

Nice work. I set it to 10 on the -jumpstart variant just to err on the side of caution, people can always lower it if they want.

For the floor level, no idea, I didn't fix it and I haven't read any changes that I can think of that would, but possible another change had a hidden effect that did. Either way, if it's fixed it's fixed, I guess.