On Wed, May 02, 2018 at 11:52:18AM +0200, Stefan Sperling wrote:
> On Wed, May 02, 2018 at 12:30:30PM +0300, Paul Irofti wrote:
> > On Mon, Apr 30, 2018 at 10:55:22AM +0200, Stefan Sperling wrote:
> > > +         /*
> > > +          * During a scan on 5Ghz, prefer RSSI measured for probe
> > > +          * response frames. i.e. don't allow beacons to lower the
> > > +          * measured RSSI. Some 5GHz APs send beacons with much
> > > +          * less Tx power than they use for probe responses.
> > > +          */
> > > +          if (isprobe)
> > 
> > Properly indent the if clause here
> > 
> > > +                 ni->ni_rssi = rxi->rxi_rssi;
> > > +         else if (ni->ni_rssi < rxi->rxi_rssi)
> > 
> > Can't this be an OR in the former if clause?
> 
> Yes it could.
> 
> > > +                 ni->ni_rssi = rxi->rxi_rssi;
> > > + } else
> > > +         ni->ni_rssi = rxi->rxi_rssi;
> > 
> > And actually, can't all of this be turned into a single if clause? :)
> > Maybe I am reading this wrong, but aren't you setting everywhere
> > ni->ni_rssi to rxi->rxi_rssi?
> 
> No. I don't update ni_rssi if the frame is a beacon (isprobe == false)
> which reduces an already recorded rssi value.
> 
> > I am a bit confused why this did not work before (when you were setting
> > the value to rxi_rssi no matter what) and why this extra checking fixed
> > it.
> 
> It didn't always work before because a beacon is not a probe response.
> Both types of frames contain the same data but semantics are different:
> The AP sends beacons at a fixed interval, and a probe response only
> when it has received a probe request from us.
> Sending probe requests during a scan is also known as an "active scan"
> (client performs an active tranmission), as opposed to a "passive scan"
> which only listens for beacons (client makes no tranmission).
> 
> The issue observed is that this AP is sending beacons with a much
> lower amount of transmit power than probe responses.

Got it, thanks!

I tried to contract the if clauses,

       if (!(ic->ic_state == IEEE80211_S_SCAN &&
           IEEE80211_IS_CHAN_5GHZ(ni->ni_chan) &&
           (isprobe || ni->ni_rssi < rxi->rxi_rssi)))
                ni->ni_rssi = rxi->rxi_rssi;

but I think what you have now is most readable.

OK pirofti@

Reply via email to