Closed ewertonsalvador closed 5 years ago
akeranen
commented
5 years ago Thank you @ewertonsalvador! I didn't quite understand yet why it takes so many movement iterations for the bug to manifest itself, but indeed your proposal seems to fix that and I don't see any harmful side effects, so I think this is good to go.
ewertonsalvador
commented
5 years ago Cheers @akeranen! It's always a pleasure to contribute! The original bug only occurred when the method setNextWaypoint (line 396 of DTNHost.java) returned false, which means that there are no more preloaded coordinates in Path. Since the number of preloaded coordinates was initialized with the constant 10 (line 49 of ExternalMovement.java), Path will only run out of preloaded coordinates (which is the condition for the bug to occur) with at least 11 coordinates in the trace file.
In the current implementation, there is a bug that appears when someone uses a movement trace file with ExternalMovement: at some point the positions of the nodes won't match the positions provided in the trace file. This behavior starts to happen when the destination property is not null and the method setNextWaypoint (line 396) returns false (meaning no more waypoints are available). In this situation, the next time the DTNHost.move method is called there will be no update to the node's next waypoint, since the destination property is different than null, preventing the simulator to run the method setNextWaypoint (line 384). The consequence of this is the occurrence of wrong values for the destination and speed properties, resulting in the simulator calculating incorrect positions for the node.
In this pull request, a correction attempt was made to DTNHost.java, and ExternalMovementTest was extended to be able to replicate the bug described above.