Command Sequence Repeating Write_Conversion

[WalterWhite1958]

Describe the bug
I have used both the WRITE_CONVERSION and POLY_WRITE_CONVERSION in my command configuration file (cmd.txt) for my target. It seems that if you create and save a command sequence that uses a command with any WRITE_CONVERSION, then when you open the saved command sequence again, it will incorrectly already have done the conversion. This means that when you send the command, it will do the conversion for a second time. **NOTE: It only seems to do this premature conversion when you open a saved command sequence, but COSMOS does not do this premature conversion when you use Command Sender (so this problem only affects the Command Sequence tool).

To Reproduce
Steps to reproduce the behavior:

1. Using the Demo, open inst_cmds.txt in the cmd_tlm folder for the target INST.
2. Add the lines:
   "SELECT_PARAMETER VALUE3
   POLY_WRITE_CONVERSION 0 2"
   under COMMAND INST SETPARAMS
3. Create a command sequence using INST SETPARAMS
4. Change VALUE3 to 1
5. Save this command sequence
6. Close the Command Sequence tool (optional)
7. Reopen the Command Sequence tool (optional)
8. Open your saved command sequence
9. Expand the command sequence and you will see that VALUE3 already got converted and is now 2.
10. Click the "Start" button to execute the command sequence
11. Inspect the raw byte packet of the command sent for SETPARAMS and you will see that a value of 4 was actually sent.

Expected behavior
So for example, let's say that you put "POLY_WRITE_CONVERSION 0 2", which means that you want to multiply the value by 2. Let's say that you want to use a value of 1, you create a command sequence that uses this command and you save the sequence using the value of 1.

What you would expect: Next time you open up the saved sequence in Command Sequence, you see the value of 1 in your saved sequence. When you execute the command sequence, it will send the value as 2 (because 2x1 = 2).

What is actually happening: When you open up the saved sequence in Command Sequence, you see that the actual value is 2 (because it already did the conversion 2x1 = 2). So when you execute the command sequence, it does the conversion again and sends the value as 4 (because 2x2 = 4).

Additional problem: If you accidentally save the command sequence again with the incorrect VALUE3 = 2, then next time you open up the saved command sequence, VALUE3 will now be 4 (because 2x2 = 4) and if you execute the command sequence, it will send VALUE3 as 8 (because 2x4 = 8). So this means that this problem stacks if you accidentally re-save the command sequence.

**NOTE: It only seems to do this premature conversion when you open a saved command sequence, but COSMOS does not do this premature conversion when you use Command Sender (so this problem only affects the Command Sequence tool).

Screenshots
See attachments

Environment (please complete the following information):

• OS: Windows 10
• COSMOS Version 4.4.0

[ghost]

Thanks for the detailed write up! It looks like this is happening with CmdSequence loads the saved sequence.

[WalterWhite1958]

Hi Jason, yes you are correct and it is happening when CmdSequence loads the saved sequence. When you first open CmdSequence, it will do the conversion on the values/parameters with WRITE_CONVERSION and then do the conversion again when you execute the Command Sequence. It's incredibly problematic because I cannot run the Command Sequence without first fixing the converted values, which means that I cannot automate running Command Sequence. If you have any further questions for me, please feel free to post a comment as I will eagerly await a fix for this problem and I would appreciate notice about the fix as it comes out. Thanks.

Please note that this problem also occurs if you try to create a new command sequence and load in a command, it will also prematurely do the conversion. So it DOES NOT ONLY AFFECT SAVED COMMAND SEQUENCES, it affects any command that is loaded into the CmdSequence tool.

It's weird that it only occurs in the CmdSequence tool, but not in the CmdSender tool.

[ghost]

@WalterWhite1958 I have pushed a PR which is linked above. Please try it out and see if it fixes your issue. To apply this individual fix to your COSMOS installation simply copy the changed files into your local lib directory with the same paths. So you'll have <COSMOS Root Config>/lib/cosmos/tools/cmd_sequence/cmd_sequence.rb plus the other two.

[WalterWhite1958]

@jasonatball Thank you for the updates, however there is still a problem. The fix that you gave me did eliminate the erroneous premature conversion whenever you load an EXISTING saved command sequence. However, the problem remains for creating a NEW command sequence. Whenever you try to add a new command that contains a WRITE_CONVERSION, it will still prematurely do the conversion.

Here's an example:
In the COSMOS Demo, I changed the inst_cmds.txt file for the target INST to include a new target with two parameters that use POLY_WRITE_CONVERSION (see attached inst_cmds.txt file).
inst_cmds.txt

These new lines are:
"
COMMAND INST TESTCOMMAND BIG_ENDIAN "JPL Testing purpose"
<%= render "_ccsds_cmd.txt", locals: {id: 10} %>
APPEND_PARAMETER CHECKTHIS1 8 UINT 0 5 0 "Check 1 value"
POLY_WRITE_CONVERSION 0 2
APPEND_PARAMETER CHECKTHIS2 8 UINT 0 5 2 "Check 2 value"
POLY_WRITE_CONVERSION 0 2
"

testsequence.txt
So let's go into the CmdSequence tool and load up the "testsequence.txt" file that I mentioned in my initial post (see attached testsequence.txt). We see that VALUE3 is saved as 1 and CmdSequence correctly loads it as 1, which means that it is no longer doing the premature conversion that previously incorrectly loaded in VALUE3 as 2. Perfect, good fix.

But the problem remains with loading in new commands that contain any type of WRITE_CONVERSION. For example, let's go ahead and add in the new INST TESTCOMMAND as shown below.

In INST TESTCOMMAND, the default for CHECKTHIS2, is supposed to be 2, not 4 as displayed in the CmdSequence tool. So obviously, the default value for any newly added commands into the CmdSequence tool is still prematurely being converted. This was not noticed before in some of your Demo parameters because the default is typically 0 (therefore 0 * 2 = 0 and you don't notice any premature conversion).

Please note that CmdSender still loads new commands correctly (there is no premature conversion). I am only pointing to this out because I want to make sure that any fix for CmdSequence does not adversely affect the CmdSender tool.

Thanks again.

[ghost]

I'm going to do a major refactor to re-use the CmdSender implementation of the command parameters table. Thanks again for the detailed bug report.

[WalterWhite1958]

@jasonatball great that sounds good. Please let me know when you post the final fix and I'll go ahead and test it on my end. Thanks again for fixing this bug.

[ghost]

@WalterWhite1958 I pushed the final fix but it's much more complex due to the refactoring so you might want to just clone the pull request.