openenergysolutions / openfmb.adapters

OpenFMB Adapters
Apache License 2.0
3 stars 1 forks source link

Migrate to new protobuf/IDL #56

Closed jadamcrain closed 5 years ago

jadamcrain commented 5 years ago

Unknown at this time what issues might be

jadamcrain commented 5 years ago

@larrylackey @dwaynebradley Here's a diff of the October 2018 protos vs the ones from April 25th 2019:

diff.txt

As far as I can tell, nothing structurally has changed, there's some additional metadata, and the go package names have changed. Does this sound correct?

Similarly here's the trivial diff for the IDL:

idl_diff.txt

jadamcrain commented 5 years ago

This was done in PR #66.

As expected from the diffs above. the C++ proto handling code did not change at all.

@emgre Is going to update his PR for the RTI stuff to incorporate the new model to ensure that works before we close this issue.

dwaynebradley commented 5 years ago

@jadamcrain, the changes in the proto definitions are ok:

  1. Go package names needed to change because of the migration from GitHub to GitLab.
  2. The new option (uml.option_openfmb_profile) = true; was implemented to give an indicator that these and ONLY these are the message structures that should be published as OpenFMB messages. We had some folks trying to just publish a MV or CMV structure as an "OpenFMB" message which was incorrect. The IDL structures already contain a method for ensuring that only "Profile" structures are published.

The IDL changes are ok as well as it is just taking into account the new MessageOptions structure we added for 2 above.

emgre commented 5 years ago

I merged the changes in the RTI DDS branch and it works as expected.