Add DENM TS access functions#114
Conversation
|
I’ve now added the denm_ts access function implementations. |
There was a problem hiding this comment.
Pull request overview
This pull request adds initial support for DENM TS (Technical Specification) access functions to the ETSI ITS messages utilities library. The PR introduces new header files for DENM TS message manipulation alongside the existing DENM EN (European Norm) support, with significant code refactoring to share common functionality between the two variants.
Changes:
- Added DENM TS access headers and implementation files (denm_ts_access.hpp, denm_ts_access.h, denm_ts_getters.h, denm_ts_setters.h)
- Refactored common DENM functionality into shared headers (denm_getters_common.h, denm_setters_common.h)
- Added CDD v2.2.1 support (cdd_v2-2-1_getters.h, cdd_v2-2-1_setters.h)
- Added corresponding test file (test_denm_ts_access.cpp)
- Updated denm_utils.h to support both EN and TS variants
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| etsi_its_msgs_utils/test/test_access.cpp | Added includes and namespace setup for DENM TS test integration |
| etsi_its_msgs_utils/test/impl/test_denm_ts_access.cpp | New test file for DENM TS access functions with comprehensive test coverage |
| etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/denm/denm_utils.h | Updated to support both EN and TS, added header guards (with ordering issue) |
| etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/denm/denm_ts_setters.h | New TS-specific setter functions for DENM |
| etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/denm/denm_ts_getters.h | New TS-specific getter functions including extensive cause code mappings |
| etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/denm/denm_ts_access.h | Main DENM TS access header with namespace setup (missing v2.2.1 guards) |
| etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/denm/denm_setters_common.h | Extracted common setter functions shared by EN and TS |
| etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/denm/denm_setters.h | Refactored to use common setters, removed duplicated code |
| etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/denm/denm_getters_common.h | Extracted common getter functions shared by EN and TS |
| etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/denm/denm_getters.h | Refactored to use common getters, removed duplicated code |
| etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/denm/denm_access.h | Updated to include new common header guards |
| etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/cdd/cdd_v2-2-1_setters.h | New CDD v2.2.1 setter functions including WGS84 heading support |
| etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/cdd/cdd_v2-2-1_getters.h | New CDD v2.2.1 getter functions for WGS84 heading |
| etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/cdd/cdd_v2-1-1_setters.h | Refactored to move common functions to cdd_setters_common.h |
| etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/cdd/cdd_v1-3-1_setters.h | Refactored to move common functions to cdd_setters_common.h |
| etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/cdd/cdd_setters_common.h | Added common CDD setter functions (setStationId, setStationType) as templates |
| etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/cdd/cdd_getters_common.h | Updated description to include v2.2.1 |
| etsi_its_msgs_utils/include/etsi_its_msgs_utils/denm_ts_access.hpp | Public header for DENM TS access in ROS 2 projects |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/denm/denm_setters_common.h
Outdated
Show resolved
Hide resolved
etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/denm/denm_getters_common.h
Outdated
Show resolved
Hide resolved
etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/denm/denm_getters_common.h
Outdated
Show resolved
Hide resolved
etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/denm/denm_utils.h
Outdated
Show resolved
Hide resolved
etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/denm/denm_ts_getters.h
Outdated
Show resolved
Hide resolved
etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/denm/denm_ts_setters.h
Outdated
Show resolved
Hide resolved
etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/denm/denm_ts_access.h
Show resolved
Hide resolved
etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/denm/denm_ts_getters.h
Outdated
Show resolved
Hide resolved
etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/denm/denm_ts_getters.h
Outdated
Show resolved
Hide resolved
etsi_its_msgs_utils/include/etsi_its_msgs_utils/impl/denm/denm_ts_getters.h
Outdated
Show resolved
Hide resolved
|
All comments have been addressed. |
| * @return WGS Heading value in degree as decimal number | ||
| */ | ||
| template <typename Wgs84Angle> | ||
| inline double getWGSHeadingCDD(const Wgs84Angle& heading) { return ((double)heading.value.value) * 1e-1; } |
There was a problem hiding this comment.
Is there a specific reason for this CDD postfix in the function-name? I think we're not having these for the ohter setter/getter functions?
There was a problem hiding this comment.
The CDD postfix is already used for low-level CDD functions (e.g. getHeadingCDD(), setHeadingCDD(), getYawRateCDD() in cdd_*_common.h). The message-level access functions stay without postfix. getWGSHeadingCDD() follows the same pattern.
| */ | ||
|
|
||
| template <typename Wgs84Angle> | ||
| inline double getWGSHeadingConfidenceCDD(const Wgs84Angle& heading) { return ((double)heading.confidence.value) * 1e-1 / etsi_its_msgs::ONE_D_GAUSSIAN_FACTOR; } |
| * @param confidence standard deviation of heading in degree as decimal number (default: infinity, mapping to Wgs84AngleConfidence::UNAVAILABLE) | ||
| */ | ||
| template <typename Wgs84Angle, typename Wgs84AngleConfidence = decltype(Wgs84Angle::confidence)> | ||
| void setWGSHeadingCDD(Wgs84Angle& heading, const double value, double confidence = std::numeric_limits<double>::infinity()) { |
| #undef ETSI_ITS_MSGS_UTILS_IMPL_CDD_CDD_V2_1_1_GETTERS_H | ||
| #undef ETSI_ITS_MSGS_UTILS_IMPL_CDD_CDD_V2_1_1_SETTERS_H |
There was a problem hiding this comment.
To my understanding DENM EN uses CDD v1.3.1 and DENM TS (Release 2) is based on CDD v2.2.1 so why are we replacing the undef of v2.1.1 with v2.2.1? I guess we should remove the "undef" for v.1.3.1 and keep both undefs vor v2.1.X instead?!
At least this should be aligned to what happens in denm_ts_access.h
There was a problem hiding this comment.
You're right. I’ve updated the undefs in both denm_acess.h and denm_ts_access.h.
| * @return WGS Heading value in degree as decimal number | ||
| */ | ||
| template <typename Wgs84Angle> | ||
| inline double getWGSHeadingCDD(const Wgs84Angle& heading) { return ((double)heading.value.value) * 1e-1; } |
There was a problem hiding this comment.
Is there a specific reason for this CDD postfix? I think we're not having these for the
| #undef ETSI_ITS_MSGS_UTILS_IMPL_CDD_CDD_V1_3_1_GETTERS_H | ||
| #undef ETSI_ITS_MSGS_UTILS_IMPL_CDD_CDD_V1_3_1_SETTERS_H | ||
| #undef ETSI_ITS_MSGS_UTILS_IMPL_CDD_CDD_V2_2_1_GETTERS_H | ||
| #undef ETSI_ITS_MSGS_UTILS_IMPL_CDD_CDD_V2_2_1_SETTERS_H |
There was a problem hiding this comment.
so we undef v1.3.1 and v2.2.1 because DENM TS is based on v2.1.1 so in denm_access.h we should undef v2.1.1 and v2.2.1 based on my understanding
There was a problem hiding this comment.
DENM TS is based on v2.2.1 so I’ve updated this and now we undef v1.3.1 and v2.1.1.
| * @param denm DENM to get the causeCode value from | ||
| * @return causeCode value | ||
| */ | ||
| inline uint8_t getCauseCodeV2(const DENM& denm) { return denm.denm.situation.event_type.cc_and_scc.choice; } |
There was a problem hiding this comment.
Same as with the CDD postfix: Do we really need this postfix? Shouldn't this be solved by the different namespaces etsi_its_denm_msgs::access for release 1 and etsi_its_denm_ts_msgs::access for release 2?
There was a problem hiding this comment.
I initially used the V2 suffix because the field type is named CauseCodeV2, but you are correct that the namespaces already distinguish EN vs TS. I've renamed these functions to keep the naming consistent.
| @@ -0,0 +1,77 @@ | |||
| #include <gtest/gtest.h> | |||
| #include <cmath> | |||
|
|
|||
There was a problem hiding this comment.
I think we're also lacking this tests for DENM EN but we might add some exemplary for the (Sub)CauseCode setters/getters
There was a problem hiding this comment.
Good point. I added some exemplary tests for the (Sub)CauseCode functions.
consistency in DENM TS access functions.
This draft PR adds the initial headers for the denm_ts access functions.
The function implementations are still pending and will be added gradually.