-
Notifications
You must be signed in to change notification settings - Fork 6
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
SATF/SASF Grammar support #1574
base: develop
Are you sure you want to change the base?
Conversation
e032b90
to
62d14b2
Compare
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.
Syntax highlighting looks good in my testing.
Were you able to test for parse errors in all of the linked files?
return `R${time} `; | ||
case 'FROM_REQUEST_START': | ||
case 'FROM_ACTIVITY_START': | ||
// TODO: This needs to be changed to refer to the start of the request. |
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.
Are we good proceeding with this TODO?
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.
This is related to NASA-AMMOS/seq-json-schema#32
Request blocks will have a special time tag in SeqN that we can use. This is just a reminder.
sequence: string, | ||
commandDictionary: CommandDictionary | null, | ||
): string | undefined { | ||
const commands = tree.topNode.getChild('Commands')?.getChildren('Command'); |
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.
Should the literals representing token and rule names use the satfConstants?
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.
We should and I updated and used them from the satfConstants
Is there a list somewhere of remaining work? You mention that this is about 60% of the current grammar rules. |
62d14b2
to
55b6373
Compare
The remaining work is to verify that both satf/sasf sis are 100% implemented. The rules I left out are things that PSYCHE, NISAR are not using here is a rough list of unsupported steps: for, |
55b6373
to
e367371
Compare
e367371
to
37764d7
Compare
37764d7
to
46eecf9
Compare
* Converts parts of SeqN to SATF/SASF * Converts parts SATF/SASF to SeqN
46eecf9
to
7b9a181
Compare
lgtm, but will defer to @joswig for the final approval
d4d77fa
to
567d765
Compare
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.
lgtm |
Closes #1496
I added support for SATF and SASF formats, which includes:
I created a converter which will eventually be a npm plugin that can take SeqN data and generate most of the SATF/SASF content. This is because each mission has slight variation so they can use this to generate 90% of the SATF/SASF and fill in the mission specific parts in their adaptation.
There's also a way to convert a SATF/SASF file into SeqN format. Not everything is carried over as SeqN doesn't support all the features of SATF/SASF
TESTING
You can find a bunch of SATF/SASF here: https://jpl365prod-my.sharepoint.com/:f:/g/personal/shaheer_khan_jpl_nasa_gov/EplqvV9nI9ZJo6JT1KVzoVwBKZHzFNbVGXWi2qUaN6k1yg?e=zIbLEa
This is hard to test as it requires hacking the Sequence Editor to use the top editor as a
SATF/SASF
editor and the bottom editor as aSeqN
. Here is the modification I did.