Add flatbuffers component#2720
Conversation
OliverHeilwagen
requested review from
a team,
FScholPer,
antonkri,
johannes-esr and
qor-lb
as code owners
|
|
OliverHeilwagen
force-pushed
the
add_flatbuffers_component
branch
from
f727465 to
b95dda6
Compare
|
The created documentation from the pull request is available at: docu-html |
| Verification only validates the buffer structure (e.g. offsets, vtables, field boundaries), | ||
| not the correctness or integrity of the payload data. | ||
|
|
||
| .. comp_req:: Load FlatBuffers Binary File |
There was a problem hiding this comment.
I guess this should be removed and is a duplicate of comp_req__filesystem__file_io
OliverHeilwagen
force-pushed
the
add_flatbuffers_component
branch
from
b95dda6 to
2087bf4
Compare
docs/modules/baselibs/flatbuffers/docs/_assets/flatbuffers_overview.drawio.svg
Show resolved
Hide resolved
aschemmel-tech
left a comment
aschemmel-tech
left a comment
There was a problem hiding this comment.
Generally the component request is well documented. Also the related issue is correctly using the change request template, Several findings documented in inline comments to resolve still.
| @@ -0,0 +1,198 @@ | |||
| .. | |||
| # ******************************************************************************* | |||
| # Copyright (c) 2025 Contributors to the Eclipse Foundation | |||
There was a problem hiding this comment.
I think new files should have "2026"
| and the potential for configuration-related runtime errors. | ||
|
|
||
|
|
||
| Basic Functionality |
There was a problem hiding this comment.
In the template this is called "Specification" - why rename?
There was a problem hiding this comment.
Forgot about the template, headings are now aligned again.
| Basic Functionality | ||
| =================== | ||
|
|
||
| The ``flatc`` compiler shall generate code for serializing, accessing, and verifying |
There was a problem hiding this comment.
isn't this rather code for deserializing? I mean from the binary data back?
There was a problem hiding this comment.
That is the benefit of FlatBuffers it doesn't require deserialization, it basically just does some pointer hops and access the data.
|
|
||
| The FlatBuffers-Library shall provide services for serialization, zero-copy access of | ||
| FlatBuffers binary data, and structural verification of buffers. The library shall also support | ||
| loading FlatBuffers binary files and optionally provide common buffer identification and version |
There was a problem hiding this comment.
what means optionally? does it provide it or not?
There was a problem hiding this comment.
Wording should be changed to opt-in buffer identification.
On default FlatBuffers only provide a 4 character buffer identification (identifier).
The intention is to provide a small default schema for a common schema version. If this is placed at the front of the buffer (1st element), a common reader is possible that doesn't need to know the module specific details.
However the module schema need to actively select it or omit it.
| FlatBuffers binary data. Code generation shall be provided for C++ and Rust at ASIL-B level | ||
| and other languages as needed at QM level (e.g. Python). | ||
|
|
||
| The FlatBuffers-Library shall provide services for serialization, zero-copy access of |
There was a problem hiding this comment.
this sounds very much like the feature requirement, you could also just refer to this here?
|
|
||
| .. note:: | ||
| Python code generation is nice-to-have for benchmark testing (scale configurations). | ||
| It is not intended for safety certification (meta model check requires safety level ASIL-B). |
There was a problem hiding this comment.
now I understand: this is because you qualified the feature requirement as ASIL_B. Try it with setting the flatbuffers feature requirement to QM and linking to feat_req__baselibs__safety for the ASIL_B component /tool requirements.
There was a problem hiding this comment.
Ah nice, i was not aware of this yet. I will change it accordingly.
There was a problem hiding this comment.
Updated accordingly, all ASIL-B requirements now link additionally to feat_req__baselibs__safety
| :implemented: NO | ||
|
|
||
| The FlatBuffers-Library tooling shall support creation of FlatBuffers binary files from | ||
| JSON-encoded files conforming to the corresponding FlatBuffers schema. |
There was a problem hiding this comment.
let's say "provided" instead "corresponding" - because how would the tool judge?
| :implemented: NO | ||
|
|
||
| The FlatBuffers-Library tooling shall support creation of FlatBuffers binary files from | ||
| JSON-encoded files conforming to the corresponding FlatBuffers schema. |
There was a problem hiding this comment.
What happens if the JSON-encoded files do not conform to the FlatBuffers schema.
There was a problem hiding this comment.
Serialisation of the binary from the JSON fails.
The baselibs PR has a manual demo for it:
https://github.com/eclipse-score/baselibs/pull/131/changes#diff-65ac3f00480b8aaa2ad3de2e8be4860118fe1ca9f6ab1ad23fbcf9a665f115d5R69
Later it is intended to be covered by schema support checks (good cases: boundaries, equivalence; bad cases). We need to see how much we demand modules to redo such tests. Minimum what i see is e2e integration test for module specific schema (boundaries, equivalence)
| :status: invalid | ||
| :implemented: NO | ||
|
|
||
| The FlatBuffers-Library tooling shall provide a mechanism to validate FlatBuffers data against |
There was a problem hiding this comment.
It is not clear what the inputs (config.bin, config.fbs) are and what is the output (result: valid/invalid).
There was a problem hiding this comment.
I will mention them in the requirement so the expected behaviour is known.
| :status: invalid | ||
| :implemented: NO | ||
|
|
||
| The FlatBuffers-Library tooling shall provide a mechanism to check whether a new version of a |
There was a problem hiding this comment.
What is this needed for. For every SW release the config binary and genarated code is produced again. Backward compatibility would be an issue if we have the config.bin in NVM (use for read/write) and not replace it. But why should we do this?
There was a problem hiding this comment.
For the configuration use case i also don't see it necessary, but we should support typical use cases e.g. transport of size limited FlatBuffers via mw::com or other transport mediums.
For such extended use cases the evolution check will be helpful. However we can also omit it for now and introduce it, once it is requested.
| :security: YES | ||
| :realizes: wp__requirements_comp | ||
|
|
||
| FlatBuffers Tooling Requirements |
There was a problem hiding this comment.
Add @flatbuffers//:flatc to the development tool list: docs/score_tools/tools_build_development/bazel.rst
3 participants
resolves issue #1935