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

Parameter System Testing #33

Draft
wants to merge 15 commits into
base: main
Choose a base branch
from
Draft

Parameter System Testing #33

wants to merge 15 commits into from

Conversation

Jbsco
Copy link
Collaborator

@Jbsco Jbsco commented Aug 22, 2024

This draft pull request includes Python scripts which can be used with COSMOS to test parameter system functionality, namely table validate and table update commands. A mock table is generated as a byte array, and various responses are scintillated by sending commands and checking the Software_Status_Packet that is returned.


# Clear counts and data products from prior tests:
cmd("Linux_Example Ccsds_Command_Depacketizer_Instance-Reset_Counts")
cmd("Linux_Example Command_Router_Instance-Reset_Data_Products")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice. There are probably other counts you want to reset too, like in the command router.

cmd("Linux_Example Command_Router_Instance-Noop_Arg with Value 1")
wait(1.0)
# Check successful command count is 2 and that the last success was Noop_Arg with last Value 1:
check_formatted("Linux_Example Software_Status_Packet Command_Router_Instance.Command_Success_Count.Value == '2'")
Copy link
Collaborator

Choose a reason for hiding this comment

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

OK. To improve this pattern for more general use I suggest the following:

  • Is there a wait-on-value directive in COSMOS? Ideally you would just be waiting for the condition that the command success count is == 2, or timeout if it does not reach that? Waiting for a time is brittle and artificially slow. This could be an issue when we want to start running tests as fast as possible with randomized inputs.
  • We should wrap this logic into a function that does the following: 1) Check the current value of Command_Success_Count (and Command_Fail_Count too) and save them off 2) send command 3) wait for command success count to increment and return, or time out and check command fail count to see if it has incremented 4) return value of 0 for success, 1 for fail, 2 for timeout (or something like this).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably also want a function that does the opposite. If the former function was called wait_for_cmd_success we would also want one that is wait_for_cmd_failure

Copy link
Collaborator Author

@Jbsco Jbsco Aug 26, 2024

Choose a reason for hiding this comment

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

Is there a wait-on-value directive in COSMOS?

wait_check is similar to the check_formatted pattern, but adds a timeout argument. Switching to this eliminates the need for wait calls. Switched to wait_check in 9b73651.

We should wrap this logic into a function that does the following: [...]
Probably also want a function that does the opposite.

I will try to implement this in a future commit. check_exception allows for checking expressions expected to fail, although I think in all cases, if the API function call causes the script to stop then manual intervention is always required to resume or restart. This behavior may be different when not instrumenting each line.

check_formatted("Linux_Example Software_Status_Packet Command_Router_Instance.Command_Success_Count.Value == '2'")
check_formatted("Linux_Example Software_Status_Packet Command_Router_Instance.Last_Successful_Command.Id == '3'")
print("Noop_Arg success OK")
check_formatted("Linux_Example Software_Status_Packet Command_Router_Instance.Noop_Arg_Last_Value.Value == '1'")
Copy link
Collaborator

Choose a reason for hiding this comment

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

What happens if check_formatted fails? Let's talk about this behavior.

# Crc_Table
Test_Packed_Table += bytearray(struct.pack(">h", 0))
# Buffer_Length
Test_Packed_Table += bytearray(struct.pack(">h", 55))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Very cool that this works. But this is brittle in that your pattern here must match the parameter table or it will fail in hard-to-debug way. We should use the Adamant python autocoder to help here.

test/scripts/update-param-sys.py Outdated Show resolved Hide resolved
test/scripts/update-param-sys.py Outdated Show resolved Hide resolved
test/scripts/update-param-sys.py Outdated Show resolved Hide resolved
test/scripts/validate-param-sys.py Outdated Show resolved Hide resolved
test/scripts/validate-param-sys.py Outdated Show resolved Hide resolved
@@ -0,0 +1,74 @@
_low_crc = [
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would be really nice to import this direct from Adamant, instead of copying it here. Has that been tried?

Copy link
Collaborator Author

@Jbsco Jbsco Sep 10, 2024

Choose a reason for hiding this comment

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

This could be done by modifying the COSMOS compose file. Binding only the script directory and any needed utilities avoids issues with the COSMOS environment and symlinks in the broader Adamant environment. Something like this should work:

openc3-cosmos-script-runner-api:
  volumes:
    - "./../adamant_example/test/scripts:/plugins/DEFAULT/targets_modified"
    - "./../adamant/gnd/util:/plugins/DEFAULT/targets_modified"

Despite this, there still appear to be issues when trying to import the CRC utility. The script will appear in the Script Runner, and can be opened, but is unable to be imported as expected.

test/scripts/test_setup.py Show resolved Hide resolved
test/scripts/update-param-sys.py Outdated Show resolved Hide resolved
test/scripts/update-param-sys.py Outdated Show resolved Hide resolved
test/scripts/validate-param-sys.py Outdated Show resolved Hide resolved
test/scripts/validate-param-sys.py Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants