Resolve dependencies between PDUs and utils
Thank you @plata for the bug reports.
Pull requests are welcome, are you interested?
@leif81 I'm sorry but I cannot provide pull requests (at least currently). Nevertheless I hope that I've pointed out the issues and possible solutions in a way that enables others to solve them without too much trouble. I'll be happy to participate if I get the possibility to do so in the future.
Thank-you for being honest!
@plata or @leif81 - Can someone describe the cyclic dependencies? I'm not seeing them.
Maybe the wording is not quite correct. It's not that A includes B and B includes A. I was referring more to the dependencies if you look at it from a package/library perspective (i.e. utils as one library and the PDUs as another). In this case there's no clear relationship like utils links PDUs or PDUs links utils because both depend on each other. Currently, this works because everything in dis6 and utils is linked to one library. However, if you would like to build dis6 and dis7, they cannot share one utils library.
Yeah if this could be resolved it would be awesome! I would like to be able to use both if possible.
Have been looking at this issue as it has popped up in some of my work. The difficulty comes in where both dis6 and dis7 PDUs are dependent on the utils class DataStream. To create dis6 and dis7 libraries, the DataStream class is compiled twice, once for each library. The other source code in the utils directory all depend/use the dis6 PDUs. I think there are a couple of ways to solve this:
-
Copy the utils to the appropriate dis6/dis7 folder Could be put in utils subfolder under dis6 or dis7. For the DataStream class, this would create a dis6 version and a dis7 version. Not ideal since you would have to maintain both if they ever change, but this keeps the current paradigm of having two separate DIS libraries for each version. This is kind of the approach already with each of the different PDU versions anyway. Probably the least impact change.
-
Restructure organization/namespaces This prevents copying code and wraps everything in a top-level opendis namespace. This would support cross-version utilities and also remove the inherent namespace clash that is present by using the DIS namespace for both dis6 and dis7. Users of the library could mix/match dis versions if needed (don't know if this is a pro or con) depending on the use case. This could be built with a single all-encompassing library, or 3 separate libraries (opendis-core, opendis-dis6, and opendis-dis7). I'm thinking the directory structure would look something like this:
|-- src
|-- opendis
|-- DataStream.h/cpp
|-- Other source files as identified/needed
|-- dis6
|-- All dis6 PDUs
|-- utils
|-- dis6-specific utils
|-- dis7
|-- All dis7 PDUs
|-- utils
|-- dis7-specific utils (none currently)
I lean toward option 2, but I'm a bit concerned with how disruptive it might be for users. Any thoughts/recommendations? I think either are fairly straight-forward quick-fixes.
This also brings up another question that should be another issue, but what about versioning? Would be good to implement semantic versioning to account for changes like this. Looks like to-date there has been no official release version.
@wadehunk I would suggest we start with option 1. I don't like repeating code either, but there are not many changes to DataStream over time. And like you mentioned it'd be least disruptive.
If longer term we find it's becoming a pain we could re-consider the larger breaking change of option 2.