Expose p2p connection of gateways and server (#150)

[non-det-alle]

This pull request is meant to fix the problem brought up in issue #150

NetworkServerHelper::Install used to implicitly create a point-to-point connection between gateways and the network server. Specifically, this function would install a PointToPointNetDevice on gateways, then required by the ForwarderHelper application installer on gateways. Thus, if ForwarderHelper::Install was called before NetworkServerHelper::Install in a custom simulation, it would generate a zero pointer runtime error with a non-descriptive message. This can be especially difficult to debug for a new user (happened to me as well in the past).

Given that NetDevice installation is usually meant to happen in the main ns-3 simulation file, I propose to move it there. The idea is to make it explicit such that new users avoid committing the error described above. I also added clearer error messages via assertions requiring a PointToPointNetDevice to be present when calling ForwarderHelper::Install. Currently, the module only supports a p2p connection, later on we could extend this to support any type of connection between gateways and the network server.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.27%. Comparing base (6babcbe) to head (e0d0a61).
Report is 1 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #151      +/-   ##
===========================================
+ Coverage    76.15%   76.27%   +0.12%     
===========================================
  Files           69       69              
  Lines         5423     5451      +28     
===========================================
+ Hits          4130     4158      +28     
  Misses        1293     1293              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[non-det-alle]

Hello @pagmatt,

Here is a new version, I tried to make it as clean as I could get it on my own. I rebased it onto develop, but you can still easily check the latest changes because they are all in commit 9ab8421. I followed your suggestion to try and have a sort of RegisterGatewayP2P function to be used in the main simulation files. As I mentioned in my last reply, this moves a bit of complexity in the main files (see the examples) but I do not think it is too bad.

In summary, the changes proposed in this PR are meant to accomplish the following:

• Expose the P2P connection between GWs and server to prevent the bug that happened if the gateways' apps were installed before the server's app (highlighted in issue m_receiveCallback in Receive function in lora-net-device #150). I feel it is also correct in theory because you should not be forced to have a specific order of installation of independent applications on different nodes;
• Allow users to instantiate the P2P connection before installing applications, which is more in line with ns-3's way of doing things (i.e., that you should normally instantiate things in the order of nodes -> net devices -> applications, being completely separate steps);
• Clarify that the module currently only supports P2P connections internally by forcing you to use AddGatewayP2P to register gateways connected to the network server.

Other minor changes coming with the last commit are:

• removing the option to provide a NodeContainer to NetworkServerHelper. Installing the app on multiple nodes does not make a lot of sense given that you have to provide net devices created beforehand. In most cases one server is enough but people who want to play with more of them can always create different NetworkServerHelpers and register different devices/gateways to them;
• adding a data structure to help storing what's needed for gateway registration in the NS app. It makes easier to have the P2P connection in one part of your code without depending on whether the NetworkServerHelper has been created yet.

Let me know your thoughts.

[pagmatt]

Hello @pagmatt,

Here is a new version, I tried to make it as clean as I could get it on my own. I rebased it onto develop, but you can still easily check the latest changes because they are all in commit 9ab8421. I followed your suggestion to try and have a sort of RegisterGatewayP2P function to be used in the main simulation files. As I mentioned in my last reply, this moves a bit of complexity in the main files (see the examples) but I do not think it is too bad.

In summary, the changes proposed in this PR are meant to accomplish the following:

• Expose the P2P connection between GWs and server to prevent the bug that happened if the gateways' apps were installed before the server's app (highlighted in issue m_receiveCallback in Receive function in lora-net-device #150). I feel it is also correct in theory because you should not be forced to have a specific order of installation of independent applications on different nodes;
• Allow users to instantiate the P2P connection before installing applications, which is more in line with ns-3's way of doing things (i.e., that you should normally instantiate things in the order of nodes -> net devices -> applications, being completely separate steps);
• Clarify that the module currently only supports P2P connections internally by forcing you to use AddGatewayP2P to register gateways connected to the network server.

Other minor changes coming with the last commit are:

• removing the option to provide a NodeContainer to NetworkServerHelper. Installing the app on multiple nodes does not make a lot of sense given that you have to provide net devices created beforehand. In most cases one server is enough but people who want to play with more of them can always create different NetworkServerHelpers and register different devices/gateways to them;
• adding a data structure to help storing what's needed for gateway registration in the NS app. It makes easier to have the P2P connection in one part of your code without depending on whether the NetworkServerHelper has been created yet.

Let me know your thoughts.

Apologies for the late reply. Changes are ok with me, great work! I just left a minor comment

[non-det-alle]

Hello @pagmatt, I don't see your comment here

[pagmatt]

Hello @pagmatt, I don't see your comment here

My bad, the comment was still pending. You should be able to see it now