BallAerospace / COSMOS

Ball Aerospace COSMOS
https://ballaerospace.github.io/cosmos-website/
Other
360 stars 129 forks source link

WRITE_CONVERSION command parameter function being called multiple times? #325

Closed ghost closed 8 years ago

ghost commented 8 years ago

I created a WRITE_CONVERSION function to compute a command checksum to put in the secondary header. Looking at the log, when I press "send" for the command, the function seems to get called 4 times. With the first and third call returning the correct value, the 2nd and 4th returning 0.

I'm new to ruby, so it is probably a newbie mistake.

Here is my function:

require 'cosmos/conversions/conversion'
module Cosmos
   class CfeCmdXsum < Conversion
      def call(value, packet, buffer)
         value = 0xFF 
         for pos in 0..buffer.length - 1 
            value = value ^ buffer[pos].ord 
            # value = value ^ buffer[pos].chr.ord 
         end
         print "Value = ", value, “\n”
         print "Packet = ", buffer, “\n”  
         return(value)
      end
   end
end

And the log when the command is sent:

2016/09/01 13:38:29.475  INFO: Log File Opened : /vagrant/cosmos/outputs/logs/2016_09_01_13_38_29_tlm.bin
Value = 143
Packet = \x18\xA9\xC0\x00\x00\x01\x00\x00
Value = 0
Packet = \x18\xA9\xC0\x00\x00\x01\x8F\x00
Value = 143
Packet = \x18\xA9\xC0\x00\x00\x01\x00\x00
Value = 0
Packet = \x18\xA9\xC0\x00\x00\x01\x8F\x00
2016/09/01 13:38:46.241  INFO: cmd('SC SC_NOOP with CCSDS_STREAMID 6313, CCSDS_SEQUENCE 49152, CCSDS_LENGTH 1, CCSDS_CHECKSUM 0, CCSDS_FC 0')

The command definition:

COMMAND SC SC_NOOP BIG_ENDIAN "Implements the Noop command that insures the SC app is alive"
  APPEND_ID_PARAMETER CCSDS_STREAMID 16 UINT MIN_UINT16 MAX_UINT16 0x18A9 "CCSDS Packet Identification" BIG_ENDIAN
  APPEND_PARAMETER CCSDS_SEQUENCE 16 UINT MIN_UINT16 MAX_UINT16 0xC000 "CCSDS Packet Sequence Control" BIG_ENDIAN
  APPEND_PARAMETER CCSDS_LENGTH 16 UINT MIN_UINT16 MAX_UINT16 1 "CCSDS Packet Data Length" BIG_ENDIAN
  APPEND_PARAMETER CCSDS_CHECKSUM 8 UINT MIN_UINT8 MAX_UINT8 0 "CCSDS Command Checksum"
    WRITE_CONVERSION cfe_cmd_xsum.rb 
  APPEND_PARAMETER CCSDS_FC 8 UINT MIN_UINT8 MAX_UINT8 0 "CCSDS Command Function Code"

Thanks

ghost commented 8 years ago

Conversions are not a good place to calculate checksums for a few reasons. Here is how COSMOS builds up commands:

  1. First all the parameters in the command are set to their default values.
  2. Then the given parameters are written into the command in the order they were given.

The above process is actually currently happening twice, once when checking if the command is hazardous and again to build the command to actually send. (That is why you saw the conversion run 4 times). I should optimize that to only happen once. Thanks for that insight!

When calculating checksums, you need it to be done last, after everything else has already been written to the command. The process above does not guarantee that because parameters are written in the order they are specified. You could have an operational constraint to always specify the checksum field last, but that would be error prone. Also, if you did not explicitly set the checksum to 0 as you did, the conversion would have only run once during the set default values phase, and you wouldn't get the correct checksum.

Anyways, the correct solution is to set the checksum in a customized interface class. You need to subclass whatever interface you are using, override the write method to set the checksum, and then actually send the packet. Something like this:

# my_udp_interface.rb placed in your targets lib folder
# Then switch out using udp_interface.rb to my_udp_interface.rb in cmd_tlm_server.txt

require 'cosmos/interfaces/udp_interface'

module Cosmos

  class MyUdpInterface < UdpInterface

    def write(packet)
      data = packet.buffer

      checksum =  # Add code here based off of using data

      packet.write('CHECKSUM', checksum)

      super(packet)
    end

  end # class

end # module
ghost commented 8 years ago

Thanks for the fast response, That makes perfect sense.

jmthomas commented 8 years ago

I actually wrote a blog post detailing this kind of thing: http://cosmosrb.com/news/2016/08/04/custom-cosmos_interface/

ghost commented 5 years ago

Write conversions on command parameters are tricky. They work perfectly on individual items, but across items you have to think about the timing.

  1. Write conversion run when the item is written
  2. Every item gets written to its default value when the command is constructed
  3. Then given values from the user overwrite the defaults
  4. So, the conversion may run once or twice, and the whether or not the other values have been written yet is not guaranteed
  5. packet.given_values can be used to retrieve a hash of the user given values to the command. You can use that to check between items, rather than getting the other items directly