ArduinoProcessScheduler icon indicating copy to clipboard operation
ArduinoProcessScheduler copied to clipboard

Added sub-instancing example.

Open GitMoDu opened this issue 6 years ago • 7 comments

Discussion here: https://github.com/wizard97/ArduinoProcessScheduler/issues/14

GitMoDu avatar Jan 24 '18 18:01 GitMoDu

Appreciate you following through and making an example.

Looks mostly good, a few suggestions:

  • Can you instead of naming your class BlinkerProcesses, 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.

wizard97 avatar Jan 25 '18 01:01 wizard97

  1. 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.

  2. Didn't catch that part, I'll add it.

GitMoDu avatar Jan 25 '18 22:01 GitMoDu

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.

wizard97 avatar Jan 25 '18 22:01 wizard97

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?

GitMoDu avatar Jan 26 '18 11:01 GitMoDu

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.

wizard97 avatar Jan 26 '18 22:01 wizard97

Updated with clean-up.

GitMoDu avatar Feb 27 '18 14:02 GitMoDu

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.

wizard97 avatar May 23 '18 04:05 wizard97