Skip to content
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

15 incorporate asam osi utilities #16

Merged
merged 15 commits into from
Dec 20, 2024

Conversation

TimmRuppert
Copy link
Contributor

@TimmRuppert TimmRuppert commented Dec 18, 2024

Reference to a related issue in the repository
Closes #15

Add a description
Use asam-osi-utilities to support writing to mcap, osi and txth trace files.

  • Uses the writers provided by asam-osi-utilities
  • Supports more top-level message types than before
  • To avoid branching and handling the topic for MCAP, I’ve wrapped the writer function in a std::function.
  • bumped OSI to 3.7.0
  • used recursive github actions/checkout in recent v4

Note: I had also the idea to write the message without deserializing and implement something like a writeSerializedMessage into asam-osi-utilities but we need the timestamps for the MCAP anyways.

Why this should be kept in draft for now:

Take this checklist as orientation for yourself, if this PR is ready for Maintainer Review

  • My suggestion follows the governance rules.
  • All commits of this PR are signed.
  • My changes generate no errors when passing CI tests.
  • I updated all documentation (readmes incl. figures) according to my changes.
  • I have successfully implemented and tested my fix/feature locally.
  • Appropriate reviewer(s) are assigned.

Signed-off-by: Timm Ruppert <timm.ruppert@persival.de>
Signed-off-by: Timm Ruppert <timm.ruppert@persival.de>
@TimmRuppert TimmRuppert linked an issue Dec 18, 2024 that may be closed by this pull request
Signed-off-by: Timm Ruppert <timm.ruppert@persival.de>
Signed-off-by: Timm Ruppert <timm.ruppert@persival.de>
Signed-off-by: Timm Ruppert <timm.ruppert@persival.de>
Signed-off-by: Timm Ruppert <timm.ruppert@persival.de>
Signed-off-by: Timm Ruppert <timm.ruppert@persival.de>
Signed-off-by: Timm Ruppert <timm.ruppert@persival.de>
Signed-off-by: Timm Ruppert <timm.ruppert@persival.de>
@ClemensLinnhoff
Copy link
Contributor

When I don't define the file_format in my ssp, it throws an error: Unknown trace file format
Could you make "osi" the default, so when not otherwise specified, it writes a native binary osi trace?

Signed-off-by: ClemensLinnhoff <clemens.linnhoff@persival.de>
@ClemensLinnhoff ClemensLinnhoff marked this pull request as ready for review December 20, 2024 08:38
Copy link
Contributor

@ClemensLinnhoff ClemensLinnhoff left a comment

Choose a reason for hiding this comment

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

What is the default setting for compression in the mcap format?

@TimmRuppert
Copy link
Contributor Author

What is the default setting for compression in the mcap format?

Zstd with a "default" compression level. That is all part of the MCAP API default inits: https://mcap.dev/docs/cpp/r832FE362A16BB6E8

Signed-off-by: Timm Ruppert <timm.ruppert@persival.de>
…L/sl-5-6-osi-trace-file-writer into 15-incorporate-asam-osi-utilities
Signed-off-by: Timm Ruppert <timm.ruppert@persival.de>
TimmRuppert and others added 2 commits December 20, 2024 16:26
Signed-off-by: Timm Ruppert <timm.ruppert@persival.de>
Signed-off-by: ClemensLinnhoff <clemens.linnhoff@persival.de>
Copy link

Cpp-Linter Report ⚠️

Some files did not pass the configured checks!

clang-format (v14.0.0) reports: 1 file(s) not formatted
  • src/OSMP.h
clang-tidy (v14.0.0) reports: 32 concern(s)
  • src/OSMP.cpp:60:5: warning: [cppcoreguidelines-pro-type-member-init]

    uninitialized record type: 'myaddr'

        union Addrconv
        ^
  • src/OSMP.cpp:69:12: warning: [cppcoreguidelines-pro-type-union-access]

    do not access members of unions; use (boost::)variant instead

        myaddr.base.lo = lo;
               ^
  • src/OSMP.cpp:70:12: warning: [cppcoreguidelines-pro-type-union-access]

    do not access members of unions; use (boost::)variant instead

        myaddr.base.hi = hi;
               ^
  • src/OSMP.cpp:71:12: warning: [cppcoreguidelines-pro-type-reinterpret-cast]

    do not use reinterpret_cast

        return reinterpret_cast<void*>(myaddr.address);
               ^
  • src/OSMP.cpp:71:43: warning: [cppcoreguidelines-pro-type-union-access]

    do not access members of unions; use (boost::)variant instead

        return reinterpret_cast<void*>(myaddr.address);
                                              ^
  • src/OSMP.cpp:82:5: warning: [cppcoreguidelines-pro-type-member-init]

    uninitialized record type: 'myaddr'

        union Addrconv
        ^
  • src/OSMP.cpp:91:12: warning: [cppcoreguidelines-pro-type-union-access]

    do not access members of unions; use (boost::)variant instead

        myaddr.address = reinterpret_cast<unsigned long long>(ptr);
               ^
  • src/OSMP.cpp:91:22: warning: [cppcoreguidelines-pro-type-reinterpret-cast]

    do not use reinterpret_cast

        myaddr.address = reinterpret_cast<unsigned long long>(ptr);
                         ^
  • src/OSMP.cpp:92:17: warning: [cppcoreguidelines-pro-type-union-access]

    do not access members of unions; use (boost::)variant instead

        hi = myaddr.base.hi;
                    ^
  • src/OSMP.cpp:93:17: warning: [cppcoreguidelines-pro-type-union-access]

    do not access members of unions; use (boost::)variant instead

        lo = myaddr.base.lo;
                    ^
  • src/OSMP.cpp:148:18: warning: [readability-convert-member-functions-to-static]

    method 'DoStart' can be made static

    fmi2Status OSMP::DoStart(fmi2Boolean tolerance_defined, fmi2Real tolerance, fmi2Real start_time, fmi2Boolean stop_time_defined, fmi2Real stop_time)
                     ^
  • src/OSMP.cpp:153:18: warning: [readability-convert-member-functions-to-static]

    method 'DoEnterInitializationMode' can be made static

    fmi2Status OSMP::DoEnterInitializationMode()
                     ^
  • src/OSMP.cpp:177:45: warning: [readability-identifier-naming]

    invalid case style for local constant 'FORMAT_MAP'

        const std::map<std::string, FileFormat> FORMAT_MAP = {{"osi", FileFormat::OSI}, {"mcap", FileFormat::MCAP}, {"txth", FileFormat::TXTH}};
                                                ^~~~~~~~~~
                                                format_map
  • src/OSMP.cpp:214:1: warning: [cppcoreguidelines-pro-type-member-init]

    constructor does not initialize these fields: boolean_vars_, integer_vars_, real_vars_

    OSMP::OSMP(fmi2String theinstance_name,
    ^
  • src/OSMP.cpp:370:24: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

    do not use array subscript when the index is not an integer constant expression

                value[i] = real_vars_[vr[i]];
                           ^
  • src/OSMP.cpp:387:24: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

    do not use array subscript when the index is not an integer constant expression

                value[i] = integer_vars_[vr[i]];
                           ^
  • src/OSMP.cpp:404:24: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

    do not use array subscript when the index is not an integer constant expression

                value[i] = boolean_vars_[vr[i]];
                           ^
  • src/OSMP.cpp:421:24: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

    do not use array subscript when the index is not an integer constant expression

                value[i] = string_vars_[vr[i]].c_str();
                           ^
  • src/OSMP.cpp:438:13: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

    do not use array subscript when the index is not an integer constant expression

                real_vars_[vr[i]] = value[i];
                ^
  • src/OSMP.cpp:455:13: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

    do not use array subscript when the index is not an integer constant expression

                integer_vars_[vr[i]] = value[i];
                ^
  • src/OSMP.cpp:472:13: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

    do not use array subscript when the index is not an integer constant expression

                boolean_vars_[vr[i]] = value[i];
                ^
  • src/OSMP.cpp:489:13: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

    do not use array subscript when the index is not an integer constant expression

                string_vars_[vr[i]] = value[i];
                ^
  • src/TraceFileWriter.cpp:84:9: warning: [readability-qualified-auto]

    'auto mcap_writer' can be declared as 'auto *mcap_writer'

            auto mcap_writer = dynamic_cast<osi3::MCAPTraceFileWriter*>(writer_.get());
            ^~~~~
            auto *
  • src/TraceFileWriter.cpp:94:9: warning: [readability-qualified-auto]

    'auto binary_writer' can be declared as 'auto *binary_writer'

            auto binary_writer = dynamic_cast<osi3::SingleChannelBinaryTraceFileWriter*>(writer_.get());
            ^~~~~
            auto *
  • src/TraceFileWriter.cpp:103:9: warning: [readability-qualified-auto]

    'auto txth_writer' can be declared as 'auto *txth_writer'

            auto txth_writer = dynamic_cast<osi3::TXTHTraceFileWriter*>(writer_.get());
            ^~~~~
            auto *
  • src/TraceFileWriter.cpp:122:13: warning: [readability-qualified-auto]

    'auto mcap_writer' can be declared as 'auto *mcap_writer'

                auto mcap_writer = dynamic_cast<osi3::MCAPTraceFileWriter*>(writer_.get());
                ^~~~~
                auto *
  • src/TraceFileWriter.cpp:170:27: warning: [readability-identifier-naming]

    invalid case style for local variable 'path_trace_final_'

        std::filesystem::path path_trace_final_ = path_trace_folder_ / (start_time_ + "_" + type_ + "_" + osi_version_ + "_" + protobuf_version_ + "_" + std::to_string(num_frames_));
                              ^~~~~~~~~~~~~~~~~
                              path_trace_final
  • src/TraceFileWriter.h:19:5: warning: [readability-identifier-naming]

    invalid case style for enum constant 'MCAP'

        MCAP,         /**< .mcap trace file format */
        ^~~~
        kMCAP
  • src/TraceFileWriter.h:20:5: warning: [readability-identifier-naming]

    invalid case style for enum constant 'OSI'

        OSI,          /**< .osi trace file format*/
        ^~~
        kOSI
  • src/TraceFileWriter.h:21:5: warning: [readability-identifier-naming]

    invalid case style for enum constant 'TXTH'

        TXTH,         /**< .txth trace file format */
        ^~~~
        kTXTH
  • src/TraceFileWriter.h:48:10: warning: [readability-identifier-naming]

    invalid case style for private method 'setupForMessageType'

        void setupForMessageType();
             ^~~~~~~~~~~~~~~~~~~
             SetupForMessageType
  • src/TraceFileWriter.h:51:55: warning: [readability-identifier-naming]

    invalid case style for constant member 'kFileNameMessageTypeMap'

        const std::unordered_map<FileFormat, std::string> kFileNameMessageTypeMap = {{FileFormat::kUnknown, ".unknown"},
                                                          ^~~~~~~~~~~~~~~~~~~~~~~
                                                          k_file_name_message_type_map

Have any feedback or feature suggestions? Share it here.

@ClemensLinnhoff ClemensLinnhoff merged commit f543fbb into main Dec 20, 2024
5 checks passed
@ClemensLinnhoff ClemensLinnhoff deleted the 15-incorporate-asam-osi-utilities branch December 20, 2024 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Incorporate asam-osi-utilities
2 participants