EdgeLab-FHDO / Edge-Diagnostic-Platform

MIT License
0 stars 2 forks source link

Network structure #79

Closed lkh5kor closed 3 years ago

lkh5kor commented 3 years ago

The first two commits contains basic class definitions. Range check for latitude, longitude and distance calculation from latitude and longitude which would be useful later.

Pending items:

  1. Relation classes implementation.
  2. JSON string read and write implementation.
  3. Unit testing.
  4. Addition of comments to all files.

Creating pull request so as to start the review and also to discuss further steps.

alyhasansakr commented 3 years ago

@lkh5kor Is it ready for review? you requested my review but I still see a lot of TODO's in the code.

lkh5kor commented 3 years ago

@lkh5kor Is it ready for review? you requested my review but I still see a lot of TODO's in the code.

@alyhasansakr , The implementation is not yet complete. As per our previous discussion, I have created the pull request. Few of the todo are related to the additional functions like distance calculation using latitude, longitude co ordinates, range checks for latitude and longitude values... etc. which we decided to keep it as it is and later move them to different files/classes. JSON string read and write and relationship between classes are pending.

alyhasansakr commented 3 years ago

@lkh5kor Creating a pull-request is not the same as requesting a review. You should only request a review when you are done with the implementation and the code is ready to be reviewed.

lkh5kor commented 3 years ago

@alyhasansakr , As per our discussion, I have updated the changes, unit testing is also done. Documentation/Comments are also added. The pull request is now ready for review. Kindly review

Regards, Shankar

lkh5kor commented 3 years ago

@alyhasansakr , The review findings are fixed. Read from JSON String test case(loadNetworkTest) is commented as the string length is exceeding the max size required for Jackson serialization, otherwise all the other test cases are covered and assertion checks have passed. Sorry, the PR was closed by mistake, I have reopened it.

alyhasansakr commented 3 years ago

@lkh5kor that is really good, there are still a few things that we are going to improve along the way. Now you need to merge the master branch into this branch and write a new class NetworkModule that extends PlatformModule. The new module should have an output that creates a new network and an input that gives the current network.

juan-castrillon commented 3 years ago

@lkh5kor Documentation is still needed for the modules, however to start follow this:

How to create a new module

  1. In the folder src/main/java/InfrastructureManager/Modules create a new folder with the module's name
  2. Inside the new package create the module class (extending PlatformModule) and a RawData package with the raw representation of the module (extends ModuleConfigData).
  3. In src/main/java/InfrastructureManager/ModuleManagement/RawData/ModuleConfigData.java add the new raw data class as a json subtype
  4. In src/main/java/InfrastructureManager/ModuleManagement/ModuleFactory.java add a new type to the ModuleType enum
  5. In the raw representation add the new type as the type parameter
  6. Again in the factory handle the new case in the switch expression to return a new Module
  7. Implement the configure method in your new module to extract the data
lkh5kor commented 3 years ago

@alyhasansakr, @juan-castrillon, I have made the requested changes as per my understanding. Kindly review. I am not aware of the current exception handling of modules. I have modified the src\main\java\InfrastructureManager\ModuleManagement\ModuleInput.java to handle one of the JSON exceptions as it was giving me error. I think that's not the proper way to handle the exception. Kindly let me know how it is to be handled.