-
Notifications
You must be signed in to change notification settings - Fork 0
Expand advisoryMessage Byte Limit in SEMI File to Support Full TIM Region Capacity #1
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
base: develop
Are you sure you want to change the base?
Expand advisoryMessage Byte Limit in SEMI File to Support Full TIM Region Capacity #1
Conversation
dmccoystephenson
left a comment
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.
Self-review
| messagePsid Psid, -- PSID of advisory message | ||
| broadcastInst BroadcastInstructions OPTIONAL, -- Broadcast instructions | ||
| advisoryMessage OCTET STRING (SIZE(0..1400)) -- Encoded advisory message | ||
| advisoryMessage OCTET STRING (SIZE(0..7000)) -- Encoded advisory message |
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.
Note: It was not necessary to modify this constraint to get the new unit tests to pass, but this constraint appears to be coupled with the constraint above as both were set to 1400 and both were for an "Encoded advisory message"
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.
Note: The regions in these test files are duplicated (in this file it is the same region 7 times), but this shouldn't impact encoding verification.
mwodahl
left a comment
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.
I just had one small question about removing unused test cases - otherwise this looks great!
src/tests.cpp
Outdated
| TEST_CASE("Encode ASD", "[encoding]" ) { | ||
| std::cout << "=== Encode ASD ===" << std::endl; | ||
| /** |
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.
question: If this test case isn't valid, can it be removed?
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.
yeah I think it could be removed; opened #2 for this
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.
commented-out test cases have been removed!
|
Marking this as a draft for now until ASD decoding can be verified on the SDX side. |
dmccoystephenson
left a comment
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.
Note: I need to check whether messages encoded with the 1400 constraint can be decoded using the 7000 constraint, and vice versa
testing: remove commented-out test cases
Problem
Per the J2735 standard, Traveler Information Messages (TIMs) can include up to 16 regions, each with 63 nodes, totaling a maximum of 1008 nodes. This results in a large OCTET STRING for the advisoryMessage element within the AdvisorySituationData structure. Currently, if this element exceeds 1400 bytes, encoding fails due to an arbitrary limit set in the SEMI file.
Solution
The SEMI file’s constraint on advisoryMessage length has been increased from 1400 bytes to 7000 bytes. This new limit accommodates encoding for all 16 regions at full capacity.
Testing
Note
It should be noted that this fix has been applied on top of the latest changes in the USDOT develop branch, not on top of the 2025 Q1 release. Since there are minimal changes, I don't think this will be a problem when using the 2025 Q1 release version of the jpo-ode project.