It is a really nice new feature. I did not check thoroughly the proposed changes, but it looks ok in general. 
Do you have an example illustrating several different cases (e.g. 1 channel, several channels, several APs on a same channel, several APs on different channels, mobile stations, ...)?

(In reply to sebastien.deronne from comment #1)
> It is a really nice new feature. I did not check thoroughly the proposed
> changes, but it looks ok in general. 
> Do you have an example illustrating several different cases (e.g. 1 channel,
> several channels, several APs on a same channel, several APs on different
> channels, mobile stations, ...)?

I don't have an example yet since it was used only in the context of 3GPP LAA indoor scenario.  However, I agree that this would be useful as a small standalone example, so I will provide one at a later date.

Example to be provided, then ok to be pushed.

Tom, by when can you finish this up? 
I think only example needs to be written.

(In reply to sebastien.deronne from comment #4)
> Tom, by when can you finish this up? 
> I think only example needs to be written.


Not sure; will bump up its priority.

Tom, any updates? I think it was almost ready to be delivered, so I still believe it could be included in the upcoming release, right?

Dear Sir

Requesting you to please update and add this patch in upcomming NS3 release, as channel roaming is my reasearch topic and it will help me lot

regards
manish Puraswani

Created attachment 2815 [details]
modified sta-wifi-mac

Created attachment 2816 [details]
modified header file

Created attachment 2817 [details]
modifed file

Dear Sir;
I have edited attached file to support channel roaming in wifi module and now it works as follows :-

1) if activeprobing is true 
    a) the STA send probe request on each channel configured using 
       AddOperationChannel method of yans-wifi-phy
    b) it wait for minChannelTimeout to switch to next channel if no activity 
       found on current channel
    c) if some activity found it will wait till maxChnnelTimeout to collect 
       probe response.
2) if activeprobing is false i.e. passive probing
    a)it wait till ScanningTimeout on each channel to get Beacon Info.

after selecting best SNR it send association request on that channel and to get AP.

Have tested same for single sta and single AP scenario, and found working fine .

I am testing same for multi STA and multi AP scenario and will update result soon,(facing problem as initialization at same time results in probe request at same time and thus collision in channel.)

With Thanks and Regards
Manish

Created attachment 2818 [details]
example with 1sta and 1 ap

modified wifi simple infra example.

(In reply to manish puraswani from comment #8)
> Created attachment 2815 [details]
> modified sta-wifi-mac

Please include patch and tell what you did change compare to Tom's implementation.

Created attachment 2824 [details]
patch file for modification

dear sir

This is the patch file for modification done. 

Following modification has been done :-

IN sta-wifi-mac.cc:-

including new variables :- 
1)MinChannelTimeout & MaxChannelTimeout to store time duration of min channel time and max channel time during active probing
2)activityDetected :- to check some activity on channel

modification of functions 
1)excluding of m_candidateChannels.size check from StaWifiMac::SendProbeRequest (void)
2)modification in StaWifiMac::TryToEnsureAssociated (void) removal of m_bestBeaconObserved.Clear (); as it was resulting in clearing of best becon data of channel and some other improvements
3) inclusion of m_phy->SetChannelNumber (m_bestBeaconObserved.m_channelNumber)in some functions to communicate channel to physical layer
4)modification in StaWifiMac::StartScanning to check active /passive probing and wait on channel accordingly.

Inclusion of new function
1)StaWifiMac::MinScanningTimeout (void)  to check whether to wait for more time to have probeResponse or to switch to new channel

and some other minor modifications.

accordingly modification has been done in sta-wif-mac.h file

some short modification in yans-wifi-phy.cc has been done, to have proper working of GetOperationalChannelList () function

with Regards
Manish Puraswani

Created attachment 2825 [details]
patch file for modification

dear sir

This is the patch file for modification done. 

Following modification has been done :-

IN sta-wifi-mac.cc:-

including new variables :- 
1)MinChannelTimeout & MaxChannelTimeout to store time duration of min channel time and max channel time during active probing
2)activityDetected :- to check some activity on channel

modification of functions 
1)excluding of m_candidateChannels.size check from StaWifiMac::SendProbeRequest (void)
2)modification in StaWifiMac::TryToEnsureAssociated (void) removal of m_bestBeaconObserved.Clear (); as it was resulting in clearing of best becon data of channel and some other improvements
3) inclusion of m_phy->SetChannelNumber (m_bestBeaconObserved.m_channelNumber)in some functions to communicate channel to physical layer
4)modification in StaWifiMac::StartScanning to check active /passive probing and wait on channel accordingly.

Inclusion of new function
1)StaWifiMac::MinScanningTimeout (void)  to check whether to wait for more time to have probeResponse or to switch to new channel

and some other minor modifications.

accordingly modification has been done in sta-wif-mac.h file

some short modification in yans-wifi-phy.cc has been done, to have proper working of GetOperationalChannelList () function

with Regards
Manish Puraswani

Hi everyone,

I have worked with Tom to resolve this bug as a part of my GSoC 2018 project. We improved the state flow of StaWifiMac, refactored part of the code for readability, and added more comments for the documentation. We decided to left out the channel roaming feature since it will complicate the scanning procedure, i.e., for devices running on a standard with a lot of possible operational channel such as 802.11ac, the total scanning timeout can be more than 5 seconds.

The summary of StaWifiMac design improvements is as follows:
- Added a scanning state to gather beacons or probe responses. So rather than STA associating to the first beacon/probe response received, it will wait for an amount of time, then associate to the best AP (i.e., the best beacon/probe resp SNR).
- If the best AP refused STA's association request, STA will try to associate to the next best AP until the list of candidate AP exhausted, if this occur STA will finally go to REFUSED state. Note that we cannot test this behavior since ns-3 haven't implement explicit association refusal from AP.

We have prepared a sequence of 3 patches:
1. Changes on documentation: added information on how Wifi infrastructure association handled on wifi-design.rst, moved StaWifiMac state graph from source file to header file and added more information [1].
2. Refactored part of Receive function that handles updating AP information when receiving beacon, probe response, and association response packets. This changes are for readability and to prepare for patch 3 [2].
3. Implemented the scanning improvements for StaWifiMac, along with documentations and unit tests [3].

[1] https://bitbucket.org/kyuucr/ns-3-dev-lbt/commits/6179c9118c85deb5019808934c051f401032003a?at=bug2399_final
[2] https://bitbucket.org/kyuucr/ns-3-dev-lbt/commits/272843ba5443768036e275cff971bf88321c1f36?at=bug2399_final
[3] https://bitbucket.org/kyuucr/ns-3-dev-lbt/commits/9e74b57a1fc875cbe043c0560a4fbd6a8387cb52?at=bug2399_final

I would like to include this in the release, so moving to last call.

I had a quick look and I don't have objection to have this pushed for the upcoming release. Some comments below:

- Please try to declare any variable representing time using nstime, uint64_t beaconInterval is a wrong choice.

- I don't see the need to have this trace in the final version, or this trace should be more explicit about what it means wrt to AP selection:
NS_LOG_DEBUG ("SnrTag value: " << snrTag.Get());

- I see this: 

In the case when AP responded to STA's association request with a
+ *    refusal, STA will try to associate to the next best AP until the list
+ *    of candidate AP is exhausted which sends STA to Refused state.
+ *    - Note that this behavior is not currently tested since ns-3 does not
+  *     implement association refusal at present.

=> An easy way to have it implemented is by adding attributes in ApWifiMac whether it should refuse non-HT or non-VHT stations. Such options exist for example in hostapd on Linux. I don't mean to have this supported right now, but this could be added somewhere in the wiki?

(In reply to sebastien.deronne from comment #18)

I updated patch 3 according to your suggestion, here is the link to it:
https://bitbucket.org/kyuucr/ns-3-dev-lbt/commits/d3a8b0b5470fc185f72da10d9e1a1e16ec722e37

> - Please try to declare any variable representing time using nstime,
> uint64_t beaconInterval is a wrong choice.

Done, I changed the variable type to nstime.

> - I don't see the need to have this trace in the final version, or this
> trace should be more explicit about what it means wrt to AP selection:
> NS_LOG_DEBUG ("SnrTag value: " << snrTag.Get());

Agreed, I removed the debug log and also moved that SnrTag code block to the appropriate conditional block (i.e., inside if (goodBeacon && m_state == WAIT_BEACON) )
 
> - I see this: 
> 
> In the case when AP responded to STA's association request with a
> + *    refusal, STA will try to associate to the next best AP until the list
> + *    of candidate AP is exhausted which sends STA to Refused state.
> + *    - Note that this behavior is not currently tested since ns-3 does not
> +  *     implement association refusal at present.
> 
> => An easy way to have it implemented is by adding attributes in ApWifiMac
> whether it should refuse non-HT or non-VHT stations. Such options exist for
> example in hostapd on Linux. I don't mean to have this supported right now,
> but this could be added somewhere in the wiki?

Agreed, this is currently outside the scope of the patch, and I would like to implement it next time.

I had just quick look , and has one query.

This patch includes sta-wifi-mac.cc with modification
AddAttribute ("ProbeRequestTimeout", "The duration to actively probe the channel.",TimeValue (Seconds (0.05))

Is it so that the patch have single time duration for active probing (here 0.05 sec)

The standard contains two time duration one Min time and other Max time for active probing.Is it contains both?

> => An easy way to have it implemented is by adding attributes in ApWifiMac
> whether it should refuse non-HT or non-VHT stations. Such options exist for
> example in hostapd on Linux. I don't mean to have this supported right now,
> but this could be added somewhere in the wiki?

If added on the wiki (such as at https://www.nsnam.org/wiki/Project_Ideas), probably more details are needed to guide someone to the final patch (e.g. the specific API desired, and specific behaviour requested).   Do you want to add this yourself if you have a good idea of what you want to see?

(In reply to manish puraswani from comment #20)
> The standard contains two time duration one Min time and other Max time for
> active probing.Is it contains both?

No, to simplify, I only use one timeout for active scanning (ProbeRequestTimeout), and one for passive scanning (WaitBeaconTimeout). These timeouts are used to gather probe responses or beacons, respectively.

I added small changes in the log for UpdateCandidateApList function, as per Tom's suggestion, the new commit for patch 3 is here:
https://bitbucket.org/kyuucr/ns-3-dev-lbt/commits/a4973ed7f06b54a9b4f8cd33fc2572ed96002d78

(In reply to manish puraswani from comment #20)
> I had just quick look , and has one query.
> 
> This patch includes sta-wifi-mac.cc with modification
> AddAttribute ("ProbeRequestTimeout", "The duration to actively probe the
> channel.",TimeValue (Seconds (0.05))
> 
> Is it so that the patch have single time duration for active probing (here
> 0.05 sec)
> 
> The standard contains two time duration one Min time and other Max time for
> active probing.Is it contains both?

No, only one.  We should probably point this out in the documentation.  I suppose that we can say that it corresponds to MaxChannelTime since MinChannelTime is not implemented.

I think that supporting Min and Max could be considered in the future as a new feature corresponding to roaming/handoff support.

A related paper, for future reference:  https://arxiv.org/pdf/1102.5189.pdf

(In reply to Tom Henderson from comment #21)
> > => An easy way to have it implemented is by adding attributes in ApWifiMac
> > whether it should refuse non-HT or non-VHT stations. Such options exist for
> > example in hostapd on Linux. I don't mean to have this supported right now,
> > but this could be added somewhere in the wiki?
> 
> If added on the wiki (such as at https://www.nsnam.org/wiki/Project_Ideas),
> probably more details are needed to guide someone to the final patch (e.g.
> the specific API desired, and specific behaviour requested).   Do you want
> to add this yourself if you have a good idea of what you want to see?

I can add this if you want. More globally, I'd like to have at least configuration options we don't have but that are available in Linux, in this case in hostapd.

(In reply to Tom Henderson from comment #23)
> (In reply to manish puraswani from comment #20)
> > I had just quick look , and has one query.
> > 
> > This patch includes sta-wifi-mac.cc with modification
> > AddAttribute ("ProbeRequestTimeout", "The duration to actively probe the
> > channel.",TimeValue (Seconds (0.05))
> > 
> > Is it so that the patch have single time duration for active probing (here
> > 0.05 sec)
> > 
> > The standard contains two time duration one Min time and other Max time for
> > active probing.Is it contains both?
> 
> No, only one.  We should probably point this out in the documentation.  I
> suppose that we can say that it corresponds to MaxChannelTime since
> MinChannelTime is not implemented.
> 
> I think that supporting Min and Max could be considered in the future as a
> new feature corresponding to roaming/handoff support.
> 
> A related paper, for future reference:  https://arxiv.org/pdf/1102.5189.pdf

If we are not fully supporting the standard, we should indeed document it.

Tom/Muhammad, are you planning to push those changes before the build freeze?

> 
> Tom/Muhammad, are you planning to push those changes before the build freeze?

Yes, I plan to merge it sometime when I get a chance (merge the latest patch series with an additional documentation note about not supporting Min and Max distinction).

(In reply to Tom Henderson from comment #26)
> > 
> > Tom/Muhammad, are you planning to push those changes before the build freeze?
> 
> Yes, I plan to merge it sometime when I get a chance (merge the latest patch
> series with an additional documentation note about not supporting Min and
> Max distinction).

I can modify the patch if you want me to. I would like to add lines on wifi-design.rst, something like: we did not implement Min/Max channel dwelling time since it is only makes sense for channel roaming, and we currently does not support it.

pushed in changeset 13633:57d2bb5a9498