Bug 2038

Summary: Stop method does not stop next wave in WaveformGenerator
Product: ns-3 Reporter: Luis Pacheco <luisbelem>
Component: spectrumAssignee: Nicola Baldo <nicola>
Status: RESOLVED FIXED    
Severity: normal CC: ns-bugs, tomh, tommaso.pecorella
Priority: P5    
Version: pre-release   
Hardware: All   
OS: All   
Attachments: patch
Fix the stop behavior
Patch including the test
check-style used

Description Luis Pacheco 2015-01-09 11:00:50 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.
Comment 1 Tommaso Pecorella 2015-01-12 04:14:06 UTC
+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 :)
Comment 2 Tom Henderson 2015-01-15 00:48:48 UTC
Luis, would you like to work on a revised patch per Tommaso's comments?
Comment 3 Luis Pacheco 2015-01-15 07:20:11 UTC
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?
Comment 4 Tom Henderson 2015-01-15 09:52:33 UTC
(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.
Comment 5 Luis Pacheco 2015-01-15 12:11:03 UTC
Created attachment 1942 [details]
Fix the stop behavior

There it is with Tommasso's sugestions, I also developed a simple test to check it.
Comment 6 Tommaso Pecorella 2015-01-15 17:17:34 UTC
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.
Comment 7 Luis Pacheco 2015-01-15 17:25:55 UTC
Created attachment 1943 [details]
Patch including the test

Haha sorry, forgot the -N flag :)
Comment 8 Tommaso Pecorella 2015-01-15 17:42:23 UTC
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 :)
Comment 9 Luis Pacheco 2015-01-15 17:57:22 UTC
Created attachment 1944 [details]
check-style used

Done :)
Comment 10 Tommaso Pecorella 2015-01-17 03:11:31 UTC
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 :)