Skip to content

Experimental: Add Flatbuffers support #427

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 8, 2021
Merged

Conversation

pmai
Copy link
Contributor

@pmai pmai commented Oct 5, 2020

This draft PR is a placeholder to highlight the ongoing work for adding support for Flatbuffers encoding of OSI messages. This work is performed as part of the Performance & Packaging WP.

Currently it supports generation of the code for Flatbuffers encoding, by auto-mapping of OSI3 proto files. It does not yet include examples making use of this encoding. Also note that use of Flatbuffers encoding is not on-the-wire compatible with OSI3 using ProtoBuffers for obvious reasons.

@pmai pmai added this to the V3.4.0 milestone May 10, 2021
@pmai pmai added FeatureRequest Proposals which enhance the interface or add additional features. Performance&Packaging The Group in the ASAM development project working on improving OSI performance. labels May 10, 2021
@Kohda-adc4g
Copy link

Checking and testing OSI #427

Environment

Linux (Ubuntu 20.04)
Open Simulation Interface v3.3.1
ProtocolBuffers v3.14.0
FlatBuffers v2.0

How to register flatbuffers as a submodule

Move to the directory where OSI is stored and register flatbuffers with the following command.

$ cd open-simulation-interface
$ git submodule add https://github.com/google/flatbuffers.git flatbuffers

.gitmodules file is automatically created and contains the contents.
It also creates clones of flatbuffers.

Modify CMakeList.txt When I create a Makefile using CMakeList.txt presented in
#427 and compile it, the following error occurs in my environment.

$ mkdir build
$ cd build
$ cmake -D BUILD_FLATBUFFER=ON ..

error: /home/OSI/test/osi_version.proto:5: 42: error: unable to load include file: google/protobuf/descriptor.proto
make[2]: *** [CMakeFiles/open_simulation_interface_fbs_build.dir/build.make:925: osi_version.fbs] error 1
make[1]: *** [CMakeFiles/Makefile2:253: CMakeFiles/open_simulation_interface_fbs_build.dir/all] error 2
make: *** [Makefile:130: all] error 2

The reason for this error is that the Protocol Buffers installation directory cannot be found when compiling with flatc.
Therefore, modify CMakeList.txt as follows and specify the location of Potocol Buffers in flatc.
Specify the location of Protocol Buffers using the flatc option flag "-I".

Add parameter:

-I "${PROTOBUF_IMPORT_DIRS}"

*** CMakeLists.txt 2021-05-12 09:34:47.159944775 +0900
--- CMakeLists.txt.pmai 2021-05-12 09:35:32.078618500 +0900


*** 102,108 ****
set(fbs "${proto_base}.fbs")
add_custom_command(
OUTPUT "${fbs}"
! COMMAND $<TARGET_FILE:flatc> -I "${PROTOBUF_IMPORT_DIRS}" -o "${CMAKE_CURRENT_BINARY_DIR}" --proto "${CMAKE_CURRENT_SOURCE_DIR}/${proto}"
DEPENDS "${proto}" flatc
WORKING_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}"
COMMENT "Convert ${proto} to ${fbs} using flatc"
--- 102,108 ----
set(fbs "${proto_base}.fbs")
add_custom_command(
OUTPUT "${fbs}"
! COMMAND $<TARGET_FILE:flatc> -o "${CMAKE_CURRENT_BINARY_DIR}" --proto "${CMAKE_CURRENT_SOURCE_DIR}/${proto}"
DEPENDS "${proto}" flatc
WORKING_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}"
COMMENT "Convert ${proto} to ${fbs} using flatc"


*** 116,122 ****
set(fbh "${flat_base}_generated.h")
add_custom_command(
OUTPUT "include/${fbh}"
! COMMAND $<TARGET_FILE:flatc> -I "${PROTOBUF_IMPORT_DIRS}" -o "${CMAKE_CURRENT_BINARY_DIR}/include" --cpp --gen-mutable --gen-name-strings --scoped-enums "${fbs}"
DEPENDS "${FLAT_FBS}" flatc
WORKING_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}"
COMMENT "Process ${fbs} to ${fbh} using flatc"
--- 116,122 ----
set(fbh "${flat_base}_generated.h")
add_custom_command(
OUTPUT "include/${fbh}"
! COMMAND $<TARGET_FILE:flatc> -o "${CMAKE_CURRENT_BINARY_DIR}/include" --cpp --gen-mutable --gen-name-strings --scoped-enums "${fbs}"
DEPENDS "${FLAT_FBS}" flatc
WORKING_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}"
COMMENT "Process ${fbs} to ${fbh} using flatc"

I was able to generate the fbs file with this without any problems.

@stefancyliax
Copy link
Contributor

OSI CCB:

  • @pmai please look at last comment. This should be part of 3.4 if we actually want to considering the switch to flatbuffers.

@pmai pmai force-pushed the experimental/flatbuffers branch from 5697d26 to 81cde6b Compare August 25, 2021 15:05
@pmai pmai self-assigned this Aug 25, 2021
@pmai pmai marked this pull request as ready for review September 8, 2021 09:32
pmai added 3 commits September 8, 2021 13:22
Signed-off-by: Pierre R. Mai <pmai@pmsf.de>
Signed-off-by: Pierre R. Mai <pmai@pmsf.de>
Signed-off-by: Pierre R. Mai <pmai@pmsf.de>
@pmai pmai force-pushed the experimental/flatbuffers branch from 81cde6b to 5e9d0d6 Compare September 8, 2021 11:23
@pmai pmai requested a review from stefancyliax September 8, 2021 11:23
@pmai
Copy link
Contributor Author

pmai commented Sep 8, 2021

@stefancyliax this should now be up-to-date with master, and could be merged, but is missing a reviewer (can't self-review). If it builds, can be merged as is.

@stefancyliax
Copy link
Contributor

@stefancyliax this should now be up-to-date with master, and could be merged, but is missing a reviewer (can't self-review). If it builds, can be merged as is.

Thanks. Will do!

Copy link
Contributor

@stefancyliax stefancyliax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@pmai pmai merged commit 7d52b71 into master Sep 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest Proposals which enhance the interface or add additional features. Performance&Packaging The Group in the ASAM development project working on improving OSI performance.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants