Skip to content

Add automatic decoding for hashed files#284

Merged
thomas-bc merged 17 commits intonasa:develfrom
youio:automate-file-hash-decoding
Mar 2, 2026
Merged

Add automatic decoding for hashed files#284
thomas-bc merged 17 commits intonasa:develfrom
youio:automate-file-hash-decoding

Conversation

@youio
Copy link
Contributor

@youio youio commented Feb 12, 2026

Related Issue(s)
nasa/fprime#4666
Has Unit Tests (y/n)
n
Documentation Included (y/n)
n

Change Description

This PR adds a check for hashed files during EventData initialization and automatically decodes and replaces any that are found for display to the user. It also adds a new variable BUILD_DIR to the environment of the flask app to allow the new code to find the hashed files no matter where fprime-gds is called.

Rationale

As outlined in issue 4666 on the fprime repository, this change allows users to benefit from using hashed file names without needing to manually call hash-to-file whenever an assert is triggered.

@youio
Copy link
Contributor Author

youio commented Feb 16, 2026

@celskeggs Good points, I pushed some changes that I think should fix all of those issues. Thanks!

Copy link
Contributor

@celskeggs celskeggs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes! I do think this is better, but I'm not sure it's totally robust yet. We may want the F Prime core team to weigh in on how we should handle some of these things.

Copy link
Collaborator

@thomas-bc thomas-bc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is super helpful, thanks a lot! A few comments for your consideration. As @celskeggs is hinting at, I think we'll need a way for users to provide a hash file themselves and not necessarily rely on the build-artifacts detection. From there, we can re-organize a few things to make this pretty self-contained.

@thomas-bc
Copy link
Collaborator

As @celskeggs is hinting at, I think we'll need a way for users to provide a hash file themselves and not necessarily rely on the build-artifacts detection.

@celskeggs as the requestor of that feature, do you agree here? Since both dict.json and hashes.txt are in the build-artifacts, an alternative is to copy/require the entire build-artifacts when using that feature. Thoughts?

@celskeggs
Copy link
Contributor

celskeggs commented Feb 17, 2026

@thomas-bc In my context, I can definitely provide a full build-artifacts directory, or at least one that contains hashes.txt and the dictionary in the right places. No complaints about the idea of --hash-file, though. But I cannot rely on providing an entire FSW build directory, just the build-artifacts.

@timcanham
Copy link
Contributor

It's typical (and sometimes required) to deliver the flight dictionaries without the source/build tree, so there shouldn't be a dependency to decode the hashes.

@youio
Copy link
Contributor Author

youio commented Feb 21, 2026

Thanks everyone for the suggestions and feedback. @thomas-bc I implemented the --hash-file flag and integrated all of your other suggestions. The code still tries to detect the hash file automatically if not provided and will simply output the original hash code if it fails. Let me know if you find any other issues with the implementation or if there's anything else I should add. Thanks!

Copy link
Contributor

@celskeggs celskeggs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

Copy link
Collaborator

@thomas-bc thomas-bc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last round of nits and recommendations - I'm expecting to be ready to merge once we've addressed them. And thanks again!

@thomas-bc
Copy link
Collaborator

@youio if you don't have time to get through this, I am happy to help

@youio
Copy link
Contributor Author

youio commented Feb 26, 2026

@thomas-bc Hi apologies I just incorporated your requests. I ended up removing the absolute path logic like you said. I thought it might be convenient but its much trickier to implement robustly than I thought and I think the relative paths are sufficient. For the args.deployment logic I realized that calling the DetectionParser handle_arguments() in the HashFileParser directly might throw the "deployment not found" error if the user is only using the dictionary and isn't using hashed files, so I made it inherit from DictionaryParser to check if the deployment is necessary before searching for it. Let me know if that solution works or if any last changes are needed. Thanks!

Copy link
Collaborator

@thomas-bc thomas-bc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Thanks a lot!

@thomas-bc thomas-bc merged commit 2aa753c into nasa:devel Mar 2, 2026
13 checks passed
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.

4 participants