🐛 | GenericMultiTimerClass broken

[hoelger]

Version

3.5.3

Branch

future3/main

OS

Ubuntu 22.04

Pi model

Laptop

Hardware

No response

What happened?

1. https://github.com/MiczFlor/RPi-Jukebox-RFID/blob/future3/main/src/jukebox/jukebox/multitimer.py#L249
   self._function = self._callee(*self.class_args, iterations=self._iterations, **self.class_kwargs)
   Instead of modifying the function call, so that later self._function can be called, here the function is already executed. In my case the return value is None, so later the MultiTimer tries to call none, and the program dies.

2. GenericMultiTimerClass tries to pass on the paramter iterations and MultiTimer tries to pass on parameter iteration so this clashes.

Should those two classes be interchangeable or not? I don't get the value of GenericMultiTimerClass because you can build just the same thing by simply using the MultiTimer so my cleanest suggestion is: Delete the GenericMultiTimerClass.

If you want to keep it, here's a fix that worked for me, on first shallow testing:
FixGenericMultiTimerClass.patch.txt

Logs

No response

Configuration

No response

More info

No response

[hoelger]

Also see typos in the same file:
typo.patch.txt

[pabera]

Thanks @hoelger .. In which situation did you call this function. I am currently working on a PR to implement the timers. You did not use this branch, correct? #2332

[hoelger]

Ah sorry, yes, I did not thoroughly look through other issues nor the state on development. So no, I just used this functionality on top of future3/main.

[pabera]

I wanted to come back to this again. Thank you for the detailed review initially!

Thank you for the detailed review! You've identified two significant issues:

1. You're absolutely right about the function execution bug. The current code executes self._callee immediately instead of storing it for later execution. I will fix this here feat: Add Idle Shutdown Timer support #2332
2. The naming conflict between iterations/iteration parameters is indeed problematic and could cause unexpected behavior.

I disagree with your assessment about GenericMultiTimerClass as we will need this in same specific use cases (e.g. IdleShutdown). I will add some documentation to explain the classes better