🐛 Allow 64-bit values in MIPI arg encoding #698
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
elbeno
force-pushed
the
fix-mipi-arg-encoding
branch
from
c555269 to
378b488
Compare
elbeno
force-pushed
the
fix-mipi-arg-encoding
branch
2 times, most recently
from
5913ac4 to
e8f119d
Compare
lukevalenty
reviewed
Mar 3, 2025
Problem: - All runtime arguments to logs are assumed by the mipi encoder to be `std::uint32_t`. - In reality, the mipi spec supports 64-bit values as well. (See MIPI-SyS-T spec table 16.) In particular, logging a time value (from a `std::chrono::duration`) needs to account for 64 bits. Solution: - Pack integral values properly according to the spec. Anything 32 bits or less is packed as 32 bits. If it's 64-bits, it's packed that way. - The XML output strings use the appropriate (printf-like) specifiers. - The JSON output retains the C++ types, as does the generated C++. Notes: - Table 16 allows 64-bit integral values even when the catalog message subtype is *_P32. - Floating point and pointer values are supported by MIPI but not by this code. Maybe in future, if there is a use case.
elbeno
force-pushed
the
fix-mipi-arg-encoding
branch
from
e8f119d to
68b18cd
Compare
lukevalenty
approved these changes
Mar 4, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem:
std::uint32_t.std::chrono::duration) needs to account for 64 bits.Solution:
Notes: