-
Notifications
You must be signed in to change notification settings - Fork 180
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
Use uint64_t for topic id in hdf5 #1883
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
contrib/ecalhdf5/src/hdf5_helper.h
Outdated
@@ -54,8 +73,7 @@ inline std::string printHex(eCAL::experimental::measurement::base::Channel::id_t | |||
|
|||
inline eCAL::experimental::measurement::base::Channel::id_t parseHexID(std::string string_id) | |||
{ | |||
auto unsigned_value = std::stoull(string_id, 0, 16); | |||
return static_cast<eCAL::experimental::measurement::base::Channel::id_t>(unsigned_value); | |||
return std::stoull(string_id, 0, 16); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: use nullptr [modernize-use-nullptr]
return std::stoull(string_id, 0, 16); | |
return std::stoull(string_id, nullptr, 16); |
@@ -68,7 +68,7 @@ namespace eCAL | |||
|
|||
struct Channel | |||
{ | |||
using id_t = std::int64_t; | |||
using id_t = std::uint64_t; | |||
|
|||
std::string name = ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: no type named 'string' in namespace 'std' [clang-diagnostic-error]
std::string name = "";
^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Changes the underlying type of the topic ID (there is no reason for IDs to be of a signed type).
Also add missing license headers.