Seqgen use json instead of xml #177
Conversation
There was a problem hiding this comment.
🔥 This is awesome - did you do any testing to validate this works?
I expected that there would be some unit tests but the only tests I saw were empty If you have some time writing a unit test for fprime-seqgen would probably be super helpful.
You may want to update the command line arguments helptext to specify that you need to supply a JSON dictionary file.
This will also require updating the fprime integration tests here
|
@thomas-bc could you kick off unit tests to see if this broke any unit tests that we're not aware of? |
Tested on a sample deployment that ran a couple of CMD_NO_OPs. I used this project and this simple_sequence.seq.txt (without the .txt). I could make the test case use a dictionary.json from a fresh deployment and generate a sequence binary off from it, but it might be a "circular" self-validating test. Any suggestions? |
|
Yeah, looking at the code structure the only straightforward way to test without refactoring is to provide a simple dictionary and input sequence and make sure it matches an expected output sequence binary. It feels a bit a circular, but does serve as a good regression test that python and other GDS infrastructure updates don't break the sequence compilation process. |
|
@thomas-bc do we have tests for the JSON loader? |
There was a problem hiding this comment.
LGTM. I'll let you and @Joshua-Anderson determine if tests belong in this PR or another.
There was a problem hiding this comment.
🚀 Looks good to me, thanks @Lex-ari!
Make sure to fix the spellcheck errors
Change Description
This makes fprime-seqgen uses json dictionaries instead of xml for sequencing.
Small typo fixes.
Rationale
Current implementation breaks when submitting a json either through the terminal or through GDS. This small fix properly converts them into binaries (and GDS properly uploads to spacecraft).
@Joshua-Anderson @thomas-bc @timcanham