Bug 2543

*** Bug 2544 has been marked as a duplicate of this bug. ***

Created attachment 2664 [details]
Patch including fix in all the schedulers (Biljana)

Hi,

I have applied the change according to Natale's patch to all the schedulers (see schedulers_sinr.patch). I tested it and all test pass.

Natale, could you please explain why did you change sinr to be a "referenced variable"? In the patch that I did based on yours, I removed this, but I would like to confirm with you that it is ok without it.

Marco, could you please double check the patch?

If you both confirm, I think that the patch is ready to be pushed.

Cheers,
Biljana

Created attachment 2665 [details]
Patch including fix in all the schedulers


Marco, please note that I have applied the patch also for Round Robin, and this scheduler was the only one to not have NO_SINR and EstimateUlSinr function implemented, so I added to it these.

(In reply to Biljana Bojović from comment #2)
> 
> Natale, could you please explain why did you change sinr to be a "referenced
> variable"? In the patch that I did based on yours, I removed this, but I
> would like to confirm with you that it is ok without it.
> 
> Marco, could you please double check the patch?

Hi Biljana,

I used a referenced variable to update also the value inside (*itCqi).second. With your patch, you're saving the estimated sinr into a temporary variable, and then (if the condition applies) into minSinr, and after that the estimated value is destroyed (leaving NO_SINR inside (*itCqi).second.at (i)). If having two distinct object is the desidered behaviour, leaving NO_SINR inside the vector, then the patch is correct; otherwise, you should use a reference, or update the value located at (*itCqi).second.at (i), after you call EstimateSinr.

Have a nice day!

Natale

A couple of comments.
The value it seems already stored within the EstimateUlSinr, line 1335: 

// store the value
(*itCqi).second.at (rb) = estimatedSinr;

Secondly, the RR was not using the Estimate, since it is using the wideband CQIs.

Natale, thanks for reporting and suggesting the patch.
Marco, thanks for revising the patch and suggesting the final version.

The final patch is pushed into the changeset: 12415:37d38dd860e0