Skip to content
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.

Sign up for GitHub

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

Feature/block relative and notes seqn #1600

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open

Feature/block relative and notes seqn #1600

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ec3f4c4
First pass at adding block relative to the grammar
cohansen Jan 16, 2025
e04c9a0
Finished adding notes
cohansen Jan 21, 2025
f3455d7
Added note seqjson converter
cohansen Jan 21, 2025
376bcb3
Added Time1 as a param to the seqjson time conversion function
cohansen Jan 21, 2025
8bfd3a6
Added more missing block relative logic
cohansen Jan 28, 2025
254410f
Added a new timeSegments token for our seqn grammar
cohansen Jan 29, 2025
b7f17dd
Fixed some broken sasf tests that were using R rather than the new B …
cohansen Jan 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
"dependencies": {
"@fontsource/jetbrains-mono": "^5.0.19",
"@nasa-jpl/aerie-ampcs": "^1.0.5",
"@nasa-jpl/seq-json-schema": "^1.2.0",
"@nasa-jpl/seq-json-schema": "^1.3.0",
"@nasa-jpl/stellar": "^1.1.18",
"@streamparser/json": "^0.0.17",
"@sveltejs/adapter-node": "5.0.1",
Expand Down
2 changes: 2 additions & 0 deletions src/utilities/codemirror/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,13 @@ export const SeqLanguage = LRLanguage.define({
LocalDeclaration: t.namespace,
MetaEntry: t.namespace,
Model: t.namespace,
Note: t.namespace,
ParameterDeclaration: t.namespace,
Request: t.namespace,
Stem: t.keyword,
String: t.string,
TimeAbsolute: t.className,
TimeBlockRelative: t.className,
TimeComplete: t.className,
TimeEpoch: t.className,
TimeGroundEpoch: t.className,
Expand Down
3 changes: 1 addition & 2 deletions src/utilities/codemirror/satf/satf-sasf-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -808,8 +808,7 @@ function parseTimeNode(timeNode: SyntaxNode | null, text: string): string {
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.
return `R${time} `;
return `B${time} `;
case 'WAIT_PREVIOUS_END':
return `C `;
default:
Expand Down
19 changes: 16 additions & 3 deletions src/utilities/codemirror/sequence.grammar
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ commandBlock {
)+
}

step { Command | Activate | GroundBlock | GroundEvent | Load }
step { Command | Activate | GroundBlock | GroundEvent | Load | Note }

commentLine {
LineComment newLine
Expand All @@ -70,7 +70,7 @@ HardwareCommands {
commandBlock
}

TimeTag { TimeAbsolute | (TimeGroundEpoch Name { String } whiteSpace) | TimeEpoch | TimeRelative | TimeComplete }
TimeTag { TimeAbsolute | (TimeGroundEpoch Name { String } whiteSpace) | TimeEpoch | TimeRelative | TimeComplete | TimeBlockRelative }

Args {
(whiteSpace (arg))* whiteSpace?
Expand Down Expand Up @@ -120,6 +120,15 @@ commonGround {
Models?
}

Note {
TimeTag
noteDirective "(" NoteValue { String } ")"
Args
LineComment?
newLine
Metadata?
}

Request {
TimeTag
requestStartDirective "(" RequestName { String } ")"
Expand Down Expand Up @@ -178,6 +187,8 @@ Stem { !stemStart identifier }

TimeRelative { 'R'(timeSecond | timeDOY | timeHhmmss) whiteSpace}

TimeBlockRelative { 'B'$[+\-]?(timeSecond | timeDOY | timeHhmmss) whiteSpace }

TimeEpoch { 'E'$[+\-]?(timeSecond | timeDOY | timeHhmmss) whiteSpace}

TimeGroundEpoch { 'G'$[+\-]?(timeSecond | timeDOY | timeHhmmss) whiteSpace}
Expand Down Expand Up @@ -226,11 +237,12 @@ Stem { !stemStart identifier }
requestEndDirective { "@REQUEST_END" }
metadataDirective { "@METADATA" }
modelDirective { "@MODEL" }
noteDirective { "@NOTE" }
genericDirective { "@"identifier }

@precedence { newLine, whiteSpace }

@precedence{ TimeAbsolute, TimeRelative, TimeEpoch, TimeComplete, TimeGroundEpoch, Boolean, identifier }
@precedence{ TimeAbsolute, TimeRelative, TimeEpoch, TimeComplete, TimeGroundEpoch, TimeBlockRelative, Boolean, identifier }

@precedence {
LoadAndGoDirective,
Expand All @@ -249,6 +261,7 @@ Stem { !stemStart identifier }
loadDirective,
groundBlockDirective,
groundEventDirective,
noteDirective,
requestStartDirective,
requestEndDirective,
engineDirective,
Expand Down
5 changes: 4 additions & 1 deletion src/utilities/sequence-editor/from-seq-json.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import type {
StringArgument,
SymbolArgument,
Time,
Time1,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is really strange. Why is there a new Time1 type? This new RelativeBlock Time tag should be in the Time Type right?

@cartermak @shaheerk94 any thoughts on this?

Copy link
Member

Choose a reason for hiding this comment

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

It's in the types.ts...I bet it has something to do with the conditional that we added to enforce that everything except COMMAND_COMPLETE needs a tag specified? But reading through types.ts, it doesn't really make sense. I wonder if the auto-generation is struggling with that conditional logic, or maybe I just am bad at TS.

/**

 * Time object

 */

export type Time = Time1 & {

  /**

   * Relative or absolute time. Required for ABSOLUTE, BLOCK_RELATIVE, COMMAND_RELATIVE, and EPOCH_RELATIVE time types but not COMMAND_COMPLETE.

   */

  tag?: string;

  /**

   * Allowed time types: ABSOLUTE, BLOCK_RELATIVE, COMMAND_RELATIVE, EPOCH_RELATIVE, or COMMAND_COMPLETE.

   */

  type: 'ABSOLUTE' | 'BLOCK_RELATIVE' | 'COMMAND_RELATIVE' | 'EPOCH_RELATIVE' | 'COMMAND_COMPLETE';

};

export type Time1 = {

  [k: string]: unknown;

};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I was curious about this as well, but I just included it in the new logic. We could remove from the underlying library and I can update my code.

Copy link
Member

Choose a reason for hiding this comment

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

Have you checked whether it behaves correctly? It seems like all the typing uses Time1, which doesn't seem to actually enforce time restrictions at all? Or maybe there's some logic I missed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Everything seems to work correctly with my testing.

VariableDeclaration,
} from '@nasa-jpl/seq-json-schema/types';
import { quoteEscape } from '../codemirror/codemirror-utils';
Expand All @@ -26,7 +27,7 @@ import { logError } from './logger';
/**
* Transform a sequence JSON time to it's sequence string form.
*/
function seqJsonTimeToSequence(time: Time): string {
function seqJsonTimeToSequence(time: Time | Time1): string {
switch (time.type) {
case 'ABSOLUTE':
return `A${time.tag ?? ''}`;
Expand All @@ -36,6 +37,8 @@ function seqJsonTimeToSequence(time: Time): string {
return `R${time.tag ?? ''}`;
case 'EPOCH_RELATIVE':
return `E${time.tag ?? ''}`;
case 'BLOCK_RELATIVE':
return `B${time.tag ?? ''}`;
default:
return '';
}
Expand Down
6 changes: 5 additions & 1 deletion src/utilities/sequence-editor/grammar.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,7 @@ Command(Stem,Args(String),Models(Model(Variable(String),Value(String),Offset(Str
[
`Seq.Json comprehension`,
`A2024-123T12:34:56 @GROUND_BLOCK("ground_block.name") # No Args
C @NOTE("note_value")
R123T12:34:56 @GROUND_EVENT("ground_event.name") "foo" 1 2 3
A2024-123T12:34:56 @ACTIVATE("activate.name") # No Args
@ENGINE 10
Expand Down Expand Up @@ -348,12 +349,14 @@ A2024-123T12:34:56 @REQUEST_BEGIN("request2.name")
R100 CMD_3 "1 2 3"
C CMD_4 1 2 3
R100 CMD_5 "1 2 3"
B00:00:00 CMD_6 "1 2 3"
@REQUEST_END
@METADATA "foo" "bar"
`,
`
Sequence(Commands(
GroundBlock(TimeTag(TimeAbsolute),GroundName(String),Args,LineComment),
Note(TimeTag(TimeComplete),NoteValue(String)Args),
GroundEvent(TimeTag(TimeRelative),GroundName(String),Args(String,Number,Number,Number)),
Activate(TimeTag(TimeAbsolute),SequenceName(String),Args,LineComment,Engine(Number),Epoch(String)),
Activate(TimeTag(TimeRelative),SequenceName(String),Args(String,Number,Number,Number),LineComment,Engine(Number)),
Expand All @@ -379,7 +382,8 @@ Sequence(Commands(
Command(TimeTag(TimeComplete),Stem,Args(Number,Number,Number)),
Command(TimeTag(TimeRelative),Stem,Args(String)),
Command(TimeTag(TimeComplete),Stem,Args(Number,Number,Number)),
Command(TimeTag(TimeRelative),Stem,Args(String))
Command(TimeTag(TimeRelative),Stem,Args(String)),
Command(TimeTag(TimeBlockRelative),Stem,Args(String))
),
Metadata(MetaEntry(Key(String),Value(String)))
)
Expand Down
22 changes: 22 additions & 0 deletions src/utilities/sequence-editor/sequence-completion.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,13 @@ export function sequenceCompletion(
section: 'Time Tags',
type: 'keyword',
},
{
apply: 'B00:00:00 ',
info: 'Execute command at an offset from the block',
label: 'B (block relative)',
section: 'Time Tags',
type: 'keyword',
},
{
apply: 'G1 "epoch.name" ',
info: 'Add a ground epoch to a request',
Expand Down Expand Up @@ -216,6 +223,21 @@ export function sequenceCompletion(
section: 'Directives',
type: 'keyword',
},
{
apply: (view, _completion, from: number, to: number) => {
view.dispatch({
changes: {
from: Math.max(0, from + (!cursor.isAfterTimeTag || cursor.isAtSymbolBefore ? -1 : 0)),
insert: `${!cursor.isAfterTimeTag ? 'C ' : ''}@NOTE("note_value")`,
to,
},
});
},
info: 'note',
label: '@NOTE',
section: 'Directives',
type: 'function',
},
);
}

Expand Down
34 changes: 25 additions & 9 deletions src/utilities/sequence-editor/sequence-linter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -757,6 +757,7 @@ function validateTimeTags(command: SyntaxNode, text: string): Diagnostic[] {
const timeTagAbsoluteNode = timeTagNode?.getChild('TimeAbsolute');
const timeTagEpochNode = timeTagNode?.getChild('TimeEpoch') ?? timeTagNode.getChild('TimeGroundEpoch');
const timeTagRelativeNode = timeTagNode?.getChild('TimeRelative');
const timeTagBlockRelativeNode = timeTagNode?.getChild('TimeBlockRelative');

if (timeTagAbsoluteNode) {
const absoluteText = text.slice(timeTagAbsoluteNode.from + 1, timeTagAbsoluteNode.to).trim();
Expand Down Expand Up @@ -825,43 +826,58 @@ function validateTimeTags(command: SyntaxNode, text: string): Diagnostic[] {
}
}
}
} else if (timeTagRelativeNode) {
const relativeText = text.slice(timeTagRelativeNode.from + 1, timeTagRelativeNode.to).trim();
} else if (timeTagRelativeNode || timeTagBlockRelativeNode) {
let relativeText = '';
let from = -1;
let to = -1;

if (timeTagRelativeNode) {
from = timeTagRelativeNode.from;
to = timeTagRelativeNode.to;
relativeText = text.slice(from + 1, to).trim();
} else if (timeTagBlockRelativeNode) {
from = timeTagBlockRelativeNode.from;
to = timeTagBlockRelativeNode.to;
relativeText = text.slice(from + 1, to).trim();
}

goetzrrGit marked this conversation as resolved.
Show resolved Hide resolved
const isValid =
validateTime(relativeText, TimeTypes.RELATIVE) || validateTime(relativeText, TimeTypes.RELATIVE_SIMPLE);
validateTime(relativeText, TimeTypes.RELATIVE) ||
(validateTime(relativeText, TimeTypes.RELATIVE_SIMPLE) && !timeTagBlockRelativeNode);
if (!isValid) {
diagnostics.push({
actions: [],
from: timeTagRelativeNode.from,
from,
message: CustomErrorCodes.InvalidRelativeTime().message,
severity: 'error',
to: timeTagRelativeNode.to,
to,
});
} else {
if (validateTime(relativeText, TimeTypes.RELATIVE)) {
if (isTimeMax(relativeText, TimeTypes.RELATIVE)) {
diagnostics.push({
actions: [],
from: timeTagRelativeNode.from,
from,
message: CustomErrorCodes.MaxRelativeTime().message,
severity: 'error',
to: timeTagRelativeNode.to,
to,
});
} else {
if (!isTimeBalanced(relativeText, TimeTypes.EPOCH)) {
diagnostics.push({
actions: [],
from: timeTagRelativeNode.from,
from,
message: CustomErrorCodes.UnbalancedTime(getBalancedDuration(relativeText)).message,
severity: 'error',
to: timeTagRelativeNode.to,
to,
});
}
}
}
}
}
}

return diagnostics;
}

Expand Down
38 changes: 37 additions & 1 deletion src/utilities/sequence-editor/to-seq-json.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import type {
Load,
Metadata,
Model,
Note,
NumberArgument,
RepeatArgument,
Request,
Expand Down Expand Up @@ -144,6 +145,26 @@ function parseRequest(requestNode: SyntaxNode, text: string, commandDictionary:
};
}

function parseNote(stepNode: SyntaxNode, text: string): Note {
const time = parseTime(stepNode, text);

const noteValueNode = stepNode.getChild('NoteValue');
const noteValue = noteValueNode ? unquoteUnescape(text.slice(noteValueNode.from, noteValueNode.to)) : 'UNKNOWN';

const description = parseDescription(stepNode, text);
const metadata = parseMetadata(stepNode, text);
const models = parseModel(stepNode, text);

return {
description,
metadata,
models,
string_arg: noteValue,
time,
type: 'note',
};
}

function parseGroundBlockEvent(stepNode: SyntaxNode, text: string): GroundBlock | GroundEvent {
const time = parseTime(stepNode, text);

Expand Down Expand Up @@ -263,6 +284,8 @@ function parseStep(child: SyntaxNode, text: string, commandDictionary: CommandDi
case 'GroundBlock':
case 'GroundEvent':
return parseGroundBlockEvent(child, text);
case 'Note':
return parseNote(child, text);
}
// Standalone comment nodes (not descriptions of steps), are not supported in the seq.json schema
// Until a schema change is coordinated, comments will dropped while writing out seq.json.
Expand Down Expand Up @@ -428,12 +451,13 @@ function parseTime(commandNode: SyntaxNode, text: string): Time {
const timeTagCompleteNode = timeTagNode.getChild('TimeComplete');
const timeTagEpochNode = timeTagNode.getChild('TimeEpoch') || timeTagNode.getChild('TimeGroundEpoch');
const timeTagRelativeNode = timeTagNode.getChild('TimeRelative');
const timeTagBlockRelativeNode = timeTagNode.getChild('TimeBlockRelative');

if (timeTagCompleteNode) {
return { type: 'COMMAND_COMPLETE' };
}

if (!timeTagAbsoluteNode && !timeTagEpochNode && !timeTagRelativeNode) {
if (!timeTagAbsoluteNode && !timeTagEpochNode && !timeTagRelativeNode && !timeTagBlockRelativeNode) {
return { tag, type: 'ABSOLUTE' };
}

Expand Down Expand Up @@ -479,7 +503,19 @@ function parseTime(commandNode: SyntaxNode, text: string): Time {
}
return { tag, type: 'COMMAND_RELATIVE' };
}
} else if (timeTagBlockRelativeNode) {
const timeTagBlockRelativeText = text.slice(timeTagBlockRelativeNode.from + 1, timeTagBlockRelativeNode.to).trim();

if (validateTime(timeTagBlockRelativeText, TimeTypes.RELATIVE)) {
const { isNegative, days, hours, minutes, seconds, milliseconds } = getDurationTimeComponents(
parseDurationString(timeTagBlockRelativeText, 'seconds'),
);
tag = `${isNegative}${days}${days ? 'T' : ''}${hours}:${minutes}:${seconds}${milliseconds ? '.' : ''}${milliseconds}`;

return { tag, type: 'BLOCK_RELATIVE' };
}
}

return { tag, type: 'ABSOLUTE' };
}

Expand Down
Loading