Bugzilla – Bug 2038
Stop method does not stop next wave in WaveformGenerator
Last modified: 2015-01-17 03:11:31 UTC
Created attachment 1940 [details] patch When invoking the stop method in the wave-generator it doesn't stop the next wave wich is already schedulled.
+1, but with some corrections. There are two calls to Simulator::Schedule in the model. BOTH should be stored in the EventId, and be stopped when Stop is called. Moreover, the Event should be checked (and stopped) also during the DoDelete, you never know. Last but not least, the EventId makes it redundant the "m_active" variable. Think about it :)
Luis, would you like to work on a revised patch per Tommaso's comments?
I am doing it, just testing right now to see if it is working accordingly. Just one doubt, when you say to check the event in the DoDelete method don't you mean DoDispose? Is DoDelete supposed to be overridden?
(In reply to Luis Pacheco from comment #3) > I am doing it, just testing right now to see if it is working accordingly. > > Just one doubt, when you say to check the event in the DoDelete method don't > you mean DoDispose? Is DoDelete supposed to be overridden? DoDispose instead of DoDelete.
Created attachment 1942 [details] Fix the stop behavior There it is with Tommasso's sugestions, I also developed a simple test to check it.
Very good, almost perfect, but... but you forgot to include the test in the patch :P Cheers, T. (In reply to Luis Pacheco from comment #5) > Created attachment 1942 [details] > Fix the stop behavior > > There it is with Tommasso's sugestions, I also developed a simple test to > check it.
Created attachment 1943 [details] Patch including the test Haha sorry, forgot the -N flag :)
I didn't run the test, but I assume its is working (also with Valgrind). As a last comment (sorry), I'd use utils/check-style.py at max level over the modified files. There are a lot of missing spaces after the function names and such. nothing dramatic but why not ? Other than that... +1 Cheers (In reply to Luis Pacheco from comment #7) > Created attachment 1943 [details] > Patch including the test > > Haha sorry, forgot the -N flag :)
Created attachment 1944 [details] check-style used Done :)
Pushed in changeset 11139:41dffb7d58cf (with some fixes in the test) (In reply to Luis Pacheco from comment #9) > Created attachment 1944 [details] > check-style used > > Done :)