-
Notifications
You must be signed in to change notification settings - Fork 713
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
[21051] DynamicType to IDL serializer #4787
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>
Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>
Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>
Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>
* Class that represents each of the Nodes that form a Tree, including the source and the leaves. | ||
*/ | ||
template <typename Info> | ||
class TreeNode : public Node<Info> |
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.
Add tests for new container class.
std::string& output, | ||
DynamicDataJsonFormat format) noexcept; | ||
|
||
// TODO |
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.
Do
DynamicDataJsonFormat format) noexcept; | ||
|
||
// TODO | ||
std::string generate_idl_schema( |
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.
Convert to a noexcept method, passing output by reference (both string and ostream overloads), and returning retcode (that would be a propagation of a DynamicType
method return code in case of error).
return ss.str(); | ||
} | ||
|
||
std::string generate_idl_schema( |
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.
Rearrange methods implementations, by forward declarating all helper functions.
const auto ret = dyn_type->get_descriptor(type_descriptor); | ||
if (ret != RETCODE_OK) | ||
{ | ||
//throw utils::InconsistencyException("No Type Descriptor"); |
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.
Propagate error codes.
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.
And add log warnings.
case TK_ENUM: | ||
case TK_UNION: |
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.
Review implementation, some information is currently missing.
case TK_MAP: | ||
return map_kind_to_str(dyn_type); | ||
|
||
case TK_STRUCTURE: |
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.
Missing inheritance case.
case TK_BYTE: | ||
return "octet"; | ||
|
||
case TK_INT16: |
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.
Add newly supported TK_INT8 and TK_UINT8 kinds.
return type_descriptor->element_type(); | ||
} | ||
|
||
std::vector<uint32_t> container_size( |
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.
Return BoundSeq type.
std::stringstream ss; | ||
ss << "sequence<" << type_kind_to_str(internal_type); | ||
|
||
for (const auto& bound : this_sequence_size) |
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.
Review this logic.
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.
Compare bound with static_cast<uint32_t>(LENGTH_UNLIMITED)
to cover the unbounded case.
Consider extending conversions to take annotations into account. |
Description
After:
Contributor Checklist
versions.md
file (if applicable).Reviewer Checklist