openMSL / sl-2-0-traffic-participant-model-repository-template

Template repository: Clone and build your ASAM OSI traffic participant model.
Mozilla Public License 2.0
3 stars 2 forks source link

Initial Template #1

Closed ClemensLinnhoff closed 1 year ago

ClemensLinnhoff commented 1 year ago

Add a description

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

nocll commented 1 year ago

Hi @jdsika and @ClemensLinnhoff .

Thanks for putting together this template model.

I've been checking this against a similar template I was making (in a private repo). So far I found this PR to be quite complete:

One thing I did not have time to check yet: I wanted to make a local build (on my PC) of the model template and the new esmini with traffic update input. However, I see this is already working on the pipeline.

Some potential improvements:

  1. In the README, should there be some explanation of using output traffic_update.update.base.___ vs. traffic_update.internal_state.___? Usually, you would use one OR the other: either the traffic agent has it's own dynamics and computes its new position/velocity to send to the simulator, OR, the traffic agent sends control inputs (e.g., throttle, steering, ...) and a block downstream uses this to compute position/velocity. Is this template expected to cover only the first case (as currently coded) or also the second one?
  2. In the integration tests, I believe you are only deciding PASS/FAIL as EXCEUTE/NO-EXECUTE. Especially in 002 Smoke Test, it would be good to verify if the traffic agent actually followed the expected positions in the environment simulator (in this case esmini).

Both of these improvements are more "nice to have", not "must have" for this PR to be completed.

ClemensLinnhoff commented 1 year ago

Thank you for your review @nocll! I put some more explanation in the readme to describe, that the update output is used in this template. Generally there are several possibilities for traffic participants (e.g. also TrafficCommand) and we just wanted to give one example here.

For testing you are right, in the end it should be tested, if the trajectory given by the model is correct, but we would like to wait for an actual model in the sub-library, before we implement this.

github-actions[bot] commented 1 year ago

Cpp-Linter Report :warning:

Some files did not pass the configured checks!

clang-format reports: 5 file(s) not formatted - src/OSMP.h
clang-tidy reports: 34 concern(s) - src/MyTrafficParticipantModel.cpp /src/MyTrafficParticipantModel.cpp:26:21: warning: [cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers] > 13.89 is a magic number; consider replacing it with a named constant ```cpp max_velocity_ = 13.89; ^ ``` - src/MyTrafficParticipantModel.h /src/MyTrafficParticipantModel.h:44:21: warning: [cppcoreguidelines-avoid-non-const-global-variables] > variable 'private_log_file' is non-const and globally accessible, consider making it const ```h static ofstream private_log_file; ^ ``` /src/MyTrafficParticipantModel.h:76:21: warning: [cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers] > 1024 is a magic number; consider replacing it with a named constant ```h char buffer[1024]; ^ ``` /src/MyTrafficParticipantModel.h:80:27: warning: [cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers] > 1024 is a magic number; consider replacing it with a named constant ```h vsnprintf(buffer, 1024, format, arg); ^ ``` - src/OSMP.cpp /src/OSMP.cpp:50:16: warning: [cppcoreguidelines-avoid-non-const-global-variables] > variable 'private_log_file' is non-const and globally accessible, consider making it const ```cpp ofstream OSMP::private_log_file; ^ ``` /src/OSMP.cpp:60:5: warning: [cppcoreguidelines-pro-type-member-init] > uninitialized record type: 'myaddr' ```cpp union Addrconv ^ ``` /src/OSMP.cpp:69:12: warning: [cppcoreguidelines-pro-type-union-access] > do not access members of unions; use (boost::)variant instead ```cpp 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 ```cpp myaddr.base.hi = hi; ^ ``` /src/OSMP.cpp:71:12: warning: [cppcoreguidelines-pro-type-reinterpret-cast] > do not use reinterpret_cast ```cpp return reinterpret_cast(myaddr.address); ^ ``` /src/OSMP.cpp:71:43: warning: [cppcoreguidelines-pro-type-union-access] > do not access members of unions; use (boost::)variant instead ```cpp return reinterpret_cast(myaddr.address); ^ ``` /src/OSMP.cpp:82:5: warning: [cppcoreguidelines-pro-type-member-init] > uninitialized record type: 'myaddr' ```cpp union Addrconv ^ ``` /src/OSMP.cpp:91:12: warning: [cppcoreguidelines-pro-type-union-access] > do not access members of unions; use (boost::)variant instead ```cpp myaddr.address = reinterpret_cast(ptr); ^ ``` /src/OSMP.cpp:91:22: warning: [cppcoreguidelines-pro-type-reinterpret-cast] > do not use reinterpret_cast ```cpp myaddr.address = reinterpret_cast(ptr); ^ ``` /src/OSMP.cpp:92:17: warning: [cppcoreguidelines-pro-type-union-access] > do not access members of unions; use (boost::)variant instead ```cpp 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 ```cpp lo = myaddr.base.lo; ^ ``` /src/OSMP.cpp:229:18: warning: [readability-convert-member-functions-to-static] > method 'DoStart' can be made static ```cpp fmi2Status OSMP::DoStart(fmi2Boolean tolerance_defined, fmi2Real tolerance, fmi2Real start_time, fmi2Boolean stop_time_defined, fmi2Real stop_time) ^ ``` /src/OSMP.cpp:234:18: warning: [readability-convert-member-functions-to-static] > method 'DoEnterInitializationMode' can be made static ```cpp fmi2Status OSMP::DoEnterInitializationMode() ^ ``` /src/OSMP.cpp:291:18: warning: [readability-convert-member-functions-to-static] > method 'DoTerm' can be made static ```cpp fmi2Status OSMP::DoTerm() ^ ``` /src/OSMP.cpp:302:1: warning: [cppcoreguidelines-pro-type-member-init] > constructor does not initialize these fields: boolean_vars_, integer_vars_, real_vars_ ```cpp OSMP::OSMP(fmi2String theinstance_name, ^ ``` /src/OSMP.cpp:462:24: warning: [cppcoreguidelines-pro-bounds-constant-array-index] > do not use array subscript when the index is not an integer constant expression ```cpp value[i] = real_vars_[vr[i]]; ^ ``` /src/OSMP.cpp:486:24: warning: [cppcoreguidelines-pro-bounds-constant-array-index] > do not use array subscript when the index is not an integer constant expression ```cpp value[i] = integer_vars_[vr[i]]; ^ ``` /src/OSMP.cpp:503:24: warning: [cppcoreguidelines-pro-bounds-constant-array-index] > do not use array subscript when the index is not an integer constant expression ```cpp value[i] = boolean_vars_[vr[i]]; ^ ``` /src/OSMP.cpp:520:24: warning: [cppcoreguidelines-pro-bounds-constant-array-index] > do not use array subscript when the index is not an integer constant expression ```cpp value[i] = string_vars_[vr[i]].c_str(); ^ ``` /src/OSMP.cpp:537:13: warning: [cppcoreguidelines-pro-bounds-constant-array-index] > do not use array subscript when the index is not an integer constant expression ```cpp real_vars_[vr[i]] = value[i]; ^ ``` /src/OSMP.cpp:554:13: warning: [cppcoreguidelines-pro-bounds-constant-array-index] > do not use array subscript when the index is not an integer constant expression ```cpp integer_vars_[vr[i]] = value[i]; ^ ``` /src/OSMP.cpp:571:13: warning: [cppcoreguidelines-pro-bounds-constant-array-index] > do not use array subscript when the index is not an integer constant expression ```cpp boolean_vars_[vr[i]] = value[i]; ^ ``` /src/OSMP.cpp:588:13: warning: [cppcoreguidelines-pro-bounds-constant-array-index] > do not use array subscript when the index is not an integer constant expression ```cpp string_vars_[vr[i]] = value[i]; ^ ``` - src/OSMP.h /src/OSMP.h:132:21: warning: [cppcoreguidelines-avoid-non-const-global-variables] > variable 'private_log_file' is non-const and globally accessible, consider making it const ```h static ofstream private_log_file; ^ ``` /src/OSMP.h:164:21: warning: [cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers] > 1024 is a magic number; consider replacing it with a named constant ```h char buffer[1024]; ^ ``` /src/OSMP.h:168:27: warning: [cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers] > 1024 is a magic number; consider replacing it with a named constant ```h vsnprintf(buffer, 1024, format, arg); ^ ``` - src/OSMPConfig.in.h /src/OSMPConfig.in.h:7:2: error: [clang-diagnostic-error] > invalid preprocessing directive ```h #cmakedefine PUBLIC_LOGGING ^ ``` /src/OSMPConfig.in.h:8:2: error: [clang-diagnostic-error] > invalid preprocessing directive ```h #cmakedefine PRIVATE_LOG_PATH "@PRIVATE_LOG_PATH@" ^ ``` /src/OSMPConfig.in.h:9:2: error: [clang-diagnostic-error] > invalid preprocessing directive ```h #cmakedefine VERBOSE_FMI_LOGGING ^ ``` /src/OSMPConfig.in.h:10:2: error: [clang-diagnostic-error] > invalid preprocessing directive ```h #cmakedefine DEBUG_BREAKS ^ ```

Have any feedback or feature suggestions? Share it here.