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

Revert setting the default width of a serialized enumeration #37

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kheaactua
Copy link

@kheaactua kheaactua commented Jan 19, 2024

Description of change

Setting the enum width as this code did narrows the enumeration value when it has a non-default width and a deployable object is unused.

e.g. That static_cast narrows the value of the enumeration regardless what its actual literal type is. This causes serialization to fail.

I first reported this in #36

Background

In #36 I show the fidl/fdepl used to define an enum with a non-default type, and show that the generated code (by generating with core, someip, then core again, as recommended in COVESA/capicxx-core-tools#32, versions 3.2.14) has the proper enum literal type. However that line causes the deserialization to fail.

This is due to this change in OutputStream.hpp

-   writeValue(_value.value_, static_cast<EmptyDeployment *>(nullptr)); 
+   // Default enumeration width is 1 Byte
+   uint8_t value = static_cast<uint8_t>(_value);
+   writeValue(value, static_cast<EmptyDeployment *>(nullptr)); 

and was committed with the comment:

Changed the default width of a serialized enumeration from "backing type width" to 1 byte.

Impact

With this change, my deployment type is still a uint8_t, e.g. (from #36)

typedef CommonAPI::SomeIP::EnumerationDeployment<uint8_t> ColorDeployment_t;

And the Deployable is still set to nullptr on creation, which is expected behaviour according to COVESA/capicxx-someip-tools#37, though it means we skip the entire code path that deals with a deployable.

But OutputStream::writeValue(const Enumeration<Base_> &_value) is able to pass the proper value (as type information is retained) to writeValue, which fixes serialization.

Setting the width as this code did narrows the enumeration value when it
isn't set to the default and a deployment object is unused.

e.g. With that static_cast, even the enumeration is a uint32_t for
example, it would be narrowed to a uint8_t and serialization would fail.
@kheaactua
Copy link
Author

kheaactua commented Jan 19, 2024

I'm confused why there is so much code guarded with if (_depl != nullptr), but as far as I can tell there is no way for _depl to be anything but nullptr. Am I missing something? I haven't found anything in the examples, CommonAPI docs, or Franca docs yet, but I'm still looking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant