Fix incorrect specification of directory contents in read dir resp [AP-945] #1379
Conversation
This file contains 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
woodfell
changed the title
woodfell
requested review from
jungleraptor,
a team,
notoriaga and
pcrumley
as code owners
woodfell
changed the base branch from
master
to
woodfell/broken_python_sbp2json
howardfernandez
approved these changes
Nov 20, 2023
pcrumley
reviewed
Nov 20, 2023
woodfell
force-pushed
the
woodfell/broken_python_sbp2json
branch
from
34d5cc4 to
327e1e0
Compare
woodfell
force-pushed
the
woodfell/fix_incorrect_string_encoding
branch
from
6bd2179 to
d6134b3
Compare
I'm about to queue up some improvements to tests which should bump up the code coverage stats and stop the sonar cloud stage from failing on each new PR. After that I'll learn how to do new releases and get everything in |
RReichert
approved these changes
Nov 21, 2023
|
[libsbp-c] Kudos, SonarCloud Quality Gate passed!
|
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.
Description
@swift-nav/devinfra
MSG_FILEIO_READ_DIR_RESP has a string field which indicates the contents of the directory. In the SBP spec the encoding of the string is set properly (multipart) but the type of the field is incorrectly set to array. This means that the field is not actually treated as a string in the generated bindings but rather as a byte array.
This generally isn’t causing problems because this message is only used by piksi and related tools which are all using older versions of libsbp. They generally haven’t been updated to use the V4 API so this change will not actually cause any problems. But it should be fixed to enable improvements to unit tests
API compatibility
Technically this does introduce an API change, but the message is unused except in legacy contexts so there should be no issue at all
API compatibility plan
If the above is "Yes", please detail the compatibility (or migration) plan:
Should any issues come up they are easy to resolve, simply use the newly generated functions in the V4 API (
sbp_msg_fileio_read_dir_resp_contents_*) instead of directly interacting with the byte arrayJIRA Reference
https://swift-nav.atlassian.net/browse/AP-945