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

[Bug] Boolean args not supported for arguments #1407

Merged
merged 5 commits into from
Aug 5, 2024

Conversation

goetzrrGit
Copy link
Contributor

Closes #1395

Previously missing boolean argument support has been added to args, along with corresponding updates to seqJSON and seqN import/export functions. I have updated the e2e test as well

* Updated precedence so the grammar checks for booleans before identifiers to generate the tree correctly.
@goetzrrGit goetzrrGit requested a review from a team as a code owner July 29, 2024 17:11
@goetzrrGit goetzrrGit added sequencing Anything related to the sequencing domain bug Something isn't working labels Jul 29, 2024
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.

If I add the GENERIC command dictionary from /e2e-tests/data/command-dictionary.xml, create a parcel using it, and then create a sequence with C FSW_CMD_0 "ON" false 1, I'm still seeing the message in the "Selected Command" panel for the boolean_arg_0 as "Unexpected value for definition".

Also, I don't see any linting errors regardless of whether I pass false, FALSE, "FALSE", or "false". Should I be seeing any?

@goetzrrGit
Copy link
Contributor Author

@duranb should be good now. I added a boolean argument to the command panel and updated the linter

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.

Unless @joswig has anything to say about the true/false needing to also be uppercase, I'm good with this! Thanks!

@goetzrrGit goetzrrGit merged commit 0d552ac into develop Aug 5, 2024
5 checks passed
@goetzrrGit goetzrrGit deleted the bug/seq_editor_boolean_args branch August 5, 2024 15:16
JosephVolosin pushed a commit that referenced this pull request Oct 21, 2024
* Added boolean to arg grammar

* Updated precedence so the grammar checks for booleans before identifiers to generate the tree correctly.

* Import boolean args correctly to seqN

* Export boolean arg in seqJSON correctly

* Hook up boolean arg to Command Panel

* Add boolean check to linter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working sequencing Anything related to the sequencing domain
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding a boolean enum command (i.e. C FSW_CMD_0...) yields an incorrect selected command view
2 participants