I just tried that button press sequence…click twice during battery blinks, light shuts off…few minutes later comes on at full blast.
” // hold, release quickly: go to lowest level
else if (event == EV_click1_hold_release) {
set_state(steady_state, 1);
return MISCHIEF_MANAGED;”
Can someone explain to me why this is “set_state(steady_state, 1);” and not the ramp ceiling I set in the config? From my understanding the second argument for set_state is the level?
Your assumption is correct.
Either it’s a bug, or it’s as intended to always start at the lowest moon
Both the .txt manual and the flowchart give no clear definition.
But it’s working as intended. I set my ramp floor to 20 and it comes on at 20 with a long press. I’m actually trying to move lowest mode to another event, but I just don’t understand that part and why it works. 
I recently found that too. And I can tell you exactly when the bug was added… just a few days ago:
https://bazaar.launchpad.net/~toykeeper/flashlight-firmware/trunk/revision/211.1.9
I made nice_delay_ms() collapse on button event until the next loop through main(), which allows for removing the error-checking code around it… “if (! nice_delay_ms(100)) return;” becomes “nice_delay_ms(100);”, which reduces ROM use.
However, that means the rest of the code in loop() will still execute before it falls out and goes back to main(). And, here’s where the problem comes in, that code executes after changing state. So the goodnight mode’s init code runs and then the rest of the battcheck sequence completes without delays, instead of the other way around. And since they both access the same globals, it breaks.
So. Solutions include any of the following:
- Re-add the error-checking code, to abort immediately instead of just quickly falling through.
- Don’t use the globals. Use only locals instead.
- Refactor the underlying code to ensure correct order of execution.
- Switch everything to more of a preemptive multitasking style instead of cooperative… if the hardware even supports that.
For now though, I think I have a tiny patch to fix it. In fsm-events.c, inside of nice_delay_ms(), move the “process_emissions();” line down a few lines, so it’s just after the “if ((nice_delay_interrupt) …” clause instead of before it. That seems to work, because it means no more events will be processed after the delay collapse has been triggered. This way, a button press can be added to the queue, but it won’t actually be processed (and a state change won’t be processed) until the UI’s loop() function has returned.
=== modified file 'ToyKeeper/spaghetti-monster/fsm-events.c'
--- ToyKeeper/spaghetti-monster/fsm-events.c 2018-08-23 05:28:35 +0000
+++ ToyKeeper/spaghetti-monster/fsm-events.c 2018-09-01 23:05:19 +0000
@@ -160,11 +160,11 @@
_delay_loop_2(BOGOMIPS*98/100);
#endif // ifdef USE_DYNAMIC_UNDERCLOCKING
- process_emissions();
if ((nice_delay_interrupt) || (old_state != current_state)) {
//nice_delay_interrupt = 0;
return 0; // state changed; abort
}
+ process_emissions();
}
return 1;
}
Could you try this and check if anything weird happens?
Oh, totally oversaw the call to nearest_level.
It gets passed level 1 but nearest level makes it start at your ramp floor.
Will check tomorrow as it is 1:20am here.
TL;DR: I made a somewhat risky plumbing change and didn’t test it enough before merging into trunk.
It works for me, and I pushed a fix to the fsm branch, but I would appreciate extra testing before merging the fix into trunk.
“// hold (initially): go to lowest level, but allow abort for regular click
else if (event == EV_click1_press) {
set_level(nearest_level(1));
return MISCHIEF_MANAGED;
}”
You mean this? How does set_level interact with set_state?
Turbo is just this “set_state(steady_state, nearest_level(MAX_LEVEL));”
Nope, i mean set_level(…) inside the steady state function on the first entry:
Line 482 is where the passed value (in your case, 1) is limited to floor/ceil and ramping config and then aet
FWIW, that’s not actually turbo. That’s the ceiling.
nearest_level(MAX_LEVEL) returns the current ramp ceiling.
To actually reach true turbo, either the ceiling must be set to MAX_LEVEL or the user must double-click during steady_state.
Thanks! Got it! :+1:
My bad.
I guess it will take some additional hours until I completely understand all of this. Last time reading this much code is like 10 years ago. :nerd_face:
Took it apart and flashed it again, now the turbo doesn’t work, or any level above the 7135 max. Guess I effed up the driver. Inspected with microscope and didn’t see any damage from removal.
Will see if I can get a stock driver board from into-outdoor.
edit, I actually like the light limited to the max output of the 7135, just don’t know if it presents a potential for smoke and flame in this state.
bansuri, that’s an unusual failure mode. If something fails, it’s usually not the FET. Usually the thing I have the most trouble with is the switch wires and the tiny switch solder pads.
Did it at least fix the other issue it had before?
Didn’t fix the other issue, still does the battery flash every now and then. Double-click turns off the emitters but it still is “on”.
I sent an email to Hank, fingers crossed he’ll sell me a new driver. If not I’ll just use it as-is and avoid turbo.
edit: Closer examination with higher magnification revealed that I nicked a trace. Repaired and FET is working again. Human error. Hopefully that fixes the batt flash too!
Thanks!
Seems to be working, nothing weird anywhere.
That’s a great idea. It’s probably due to my ineptitude and lack of time, but I haven’t managed to flash firmware yet using macOS. I was just about ready to give up and try a Windows VM, but Ubuntu makes more sense.
Still, if anyone here is flashing drivers with macOS I’d love to hear about your setup.
I haven’t tried flashing because I don’t have any drivers to flash to. I use CrossPack from this guide AVR Tutorial - Software setup for Mac OS X and it’s much easier to install rather then installing one on Raspbian (which I failed :D) you don’t need to install anything else and can find complete documentation on how to use it.
Another bugfix today: The stepped ramping mode sometimes acted weird when configured with very specific values. Specifically, 1/146/10 as found by maukka.
This was an interesting bug, because it was only triggered by integer negative zero. The 8-bit signed integer can hold values from –128 to +127… so if you negate –128, it becomes 0.
Anyway, the stepped ramping code tries to find the level which is closest to whatever target number it was given. So it does a search, and returns the step with the smallest difference from the target level. It does this comparison by taking an absolute value of one value minus the other. Problem is, if the difference happens to be –128, the absolute value of that is zero. So it thought it found an exact match, and returned a level exactly 128 steps below what it should.
But this only happened with very specific config values. Specifically, when one ramp step happens to be exactly 128 steps away from another ramp step. But that’s not enough. It also required that the exact distance between steps happened to align with the approximated integer-math distance between steps. It would only fail if the integer math hit the bulls-eye, not if it missed the mark.
Anyway, it was an easy fix. Replace an int8_t with int16_t. But the cause of the bug was a little odd… the integer version of negative zero.
Is this the only change made in anduril.2018-09-03 ?
as i have anduril.2018-08-22 in my lights 