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.

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

Sequence Editor Ground epoch time tag #1443

Merged
merged 6 commits into from
Sep 17, 2024

Conversation

goetzrrGit
Copy link
Contributor

Closes #1368

A new time tag, 'G', to represent the ground epoch in the sequence

Reconfigured Ground Epoch: The ground epoch is now represented as a time tag 'G' to better match with existing time tags R1 A2020-001T00:00:00 E+00:10:00 C.

Autocompletion support: The 'G' time tag has been added to autocompletion suggestions.

Function refactoring: Time checking logic has been moved to a dedicated function
Linting support: Linting rules have been updated to support the 'G' time tag and ensure correct usage within 'Request' blocks.
SeqJson support: Both export and import functionality for SeqJson include the 'G' time tag, enabling seamless data exchange.
Unit test updates: Unit tests have been modified to cover the new 'G' time tag functionality.

Testing: Make sure you copy-paste this into the editor and export reimport the seqJson for round tripping

@ID "G Time Tag"

G-00:00:01 "epoch1" @REQUEST_BEGIN("request1")
  C FSW_CMD_0 "ON" false 1
@REQUEST_END

G+10 "epoch2" @REQUEST_BEGIN("request2")
  C FSW_CMD_0 "ON" false 1
@REQUEST_END

G002T00:01:00 "epoch3" @REQUEST_BEGIN("request3")
  C FSW_CMD_0 "ON" false 1
@REQUEST_END

C @REQUEST_BEGIN("request4")
  C FSW_CMD_0 "ON" false 1
@REQUEST_END


@goetzrrGit goetzrrGit requested a review from a team as a code owner August 29, 2024 15:54
@goetzrrGit goetzrrGit added the sequencing Anything related to the sequencing domain label Aug 29, 2024
@dandelany dandelany requested review from cohansen and joswig and removed request for AaronPlave August 29, 2024 17:23
@goetzrrGit goetzrrGit force-pushed the feature/ground_epoch_time_tag branch from b5e0b92 to 77fbe2b Compare September 10, 2024 14:53
Copy link
Collaborator

@duranb duranb left a comment

Choose a reason for hiding this comment

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

lgtm! pending @joswig's 👀 on the grammar

@joswig
Copy link
Collaborator

joswig commented Sep 13, 2024

I'm good with the grammar change if @shaheerk94 concurs with the resulting language

The format differed because, I thought the name should go before the delta, I'll also add that the delta is unrestricted in the schema, but downstream tools aren't as flexible and I consider that a required schema update that shouldn't block this.

https://github.com/NASA-AMMOS/seq-json-schema/blob/fdad4b3af45906de9c02a3fb67817f1e399fde69/schema.json#L151

@joswig joswig requested a review from shaheerk94 September 13, 2024 01:16
Copy link
Collaborator

@joswig joswig left a comment

Choose a reason for hiding this comment

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

@shaheerk94 Would you confirm if this lines up with #1368 sufficiently

Copy link

@shaheerk94 shaheerk94 left a comment

Choose a reason for hiding this comment

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

New format will work - agree with Chet we may need to add more restrictions in the future but that doesn't have to block this merge.

* Renamed 'cursor.isTimeTagBefore' to 'cursor.isAfterTimeTag' to make it clearer
* Moved time checking to a function
* Lint 'Request' blocks
* Added time checking for 'request'
* Support 'G' time tag linting
@goetzrrGit goetzrrGit force-pushed the feature/ground_epoch_time_tag branch from 77fbe2b to 717ea6e Compare September 17, 2024 18:15
@goetzrrGit
Copy link
Contributor Author

@dandelany

Breaking Change:

Reworked the Ground Epoch Time for the request blocks to match existing time tags (R,E,A,C)

Old Format: @GROUND_EPOCH("Name", "+1.00") @REQUEST_BEGIN
New Format: G1 "Name" @REQUEST_BEGIN

This change breaks any sequence that relies on the old way to represent ground epoch times. You will need to change @GROUND_EPOCH annotation to a G time tag as shown above.

@goetzrrGit goetzrrGit merged commit acdf979 into develop Sep 17, 2024
5 checks passed
@goetzrrGit goetzrrGit deleted the feature/ground_epoch_time_tag branch September 17, 2024 21:43
JosephVolosin pushed a commit that referenced this pull request Oct 21, 2024
* Reconfigure ground epoch into a time tag 'G'

* Added TimeTag 'G' to autocompletion

* Renamed 'cursor.isTimeTagBefore' to 'cursor.isAfterTimeTag' to make it clearer

* Added Time Tag 'G' and 'Request' linting support

* Moved time checking to a function
* Lint 'Request' blocks
* Added time checking for 'request'
* Support 'G' time tag linting

* Added Time Tag 'G' support for seqJson export

* Update unit test

* Added Time Tag 'G' support for seqJson import

* Update unit test

* Update unit test to support 'G' time tag
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sequencing Anything related to the sequencing domain
Projects
None yet
4 participants