Open GitMoDu opened 6 years ago
Appreciate you following through and making an example.
Looks mostly good, a few suggestions:
Can you instead of naming your classBlinkerProcesses
, name it something like BlinkerManager
? I saw the word process in the name and assumed it inherited from the base process which confused me for a bit.
Can you add a deconstructor to the BlinkerManager that deletes all the BlinkProcesses from the scheduler? If you don't do this and the BlinkerManager goes out of scope, the Scheduler will contain references to the now nonexistent BlinkProcesses, which would add all sorts of weird behavior and/or crashing.
Sure, it is confusing. I think I started by extending the Process class but then realized it might be more useful to have an example without direct inheritance.
Didn't catch that part, I'll add it.
I think it would actually be pretty cool if you could think of an example where this "managing process" is actually a process itself. Maybe you make the manager process change the way the LEDS blink every 10 seconds or something? That would be pretty cool.
Can you add a deconstructor to the BlinkerManager that deletes all the BlinkProcesses from the scheduler?
Do you mean just overriding the cleanUp() method, or catching the ~Process() to remove the sub-processes from the scheduler?
Definitely needs to be caught in cleanUp().
I actually realized calling cleanUp() on the child procs in the deconstructor isn't safe. As calling cleanUp() only queues the request to destroy a process. By the time the request actually gets processed the childprocs themselves will be out of scope since the deconstructor already would have run.
Updated with clean-up.
Sorry, I completely missed your updates!
Everything looks good, besides the cleanup of the child processes. I can't think of a way to do it that is clean and also safe.
Discussion here: https://github.com/wizard97/ArduinoProcessScheduler/issues/14