Merge pull request #6 from OpenC3/directive_code_part_of_data_length

Add directive code byte to data length

.github/workflows/playwright.yml

@@ -27,7 +27,7 @@ jobs:
           ref: main
       - uses: actions/setup-node@v3
         with:
-          node-version: "14"
+          node-version: "18"
           cache: "yarn"
           cache-dependency-path: "**/yarn.lock"
       - name: Install playwright dependencies

microservices/CFDP/Gemfile

@@ -2,13 +2,13 @@ source ENV['RUBYGEMS_URL'] || "https://rubygems.org"
 git_source(:github) { |repo| "https://github.com/#{repo}.git" }
 
 # Bundle edge Rails instead: gem 'rails', github: 'rails/rails'
-gem 'rails', '>= 7.0.0'
+gem 'rails', '~> 7.1.0'
 
 # Reduces boot times through caching; required in config/boot.rb
 gem 'bootsnap', '>= 1.9.3', require: false
 
 # Use Rack CORS for handling Cross-Origin Resource Sharing (CORS), making cross-origin AJAX possible
-gem 'rack-cors', '>= 1.1'
+gem 'rack-cors', '~> 2.0'
 
 # Windows does not include zoneinfo files, so bundle the tzinfo-data gem
 gem 'tzinfo-data'
@@ -21,7 +21,7 @@ group :development, :test do
 end
 
 group :test do
-  gem 'mock_redis'
+  gem 'mock_redis', '0.39'
 end
 
 if ENV['OPENC3_DEVEL']

microservices/CFDP/app/controllers/cfdp_controller.rb

@@ -33,9 +33,9 @@ def put
     transaction = $cfdp_user.start_source_transaction(params)
     render json: transaction.id
   rescue ActionController::ParameterMissing => error
-    render :json => { :status => 'error', :message => error.message }, :status => 400
+    render :json => { :status => 'error', :message => error.message }.as_json(:allow_nan => true), :status => 400
   rescue => error
-    render :json => { :status => 'error', :message => error.message }, :status => 500
+    render :json => { :status => 'error', :message => "#{error.message}\n#{error.backtrace.join("\n")}" }.as_json(:allow_nan => true), :status => 500
   end
 
   # Cancel.request (transaction ID)
@@ -46,10 +46,10 @@ def cancel
     if transaction
       render json: transaction.id
     else
-      render :json => { :status => 'error', :message => "Transaction #{params[:transaction_id]} not found" }, :status => 404
+      render :json => { :status => 'error', :message => "Transaction #{params[:transaction_id]} not found" }.as_json(:allow_nan => true), :status => 404
     end
   rescue => error
-    render :json => { :status => 'error', :message => error.message }, :status => 500
+    render :json => { :status => 'error', :message => "#{error.message}\n#{error.backtrace.join("\n")}" }.as_json(:allow_nan => true), :status => 500
   end
 
   # Suspend.request (transaction ID)
@@ -60,10 +60,10 @@ def suspend
     if transaction
       render json: transaction.id
     else
-      render :json => { :status => 'error', :message => "Transaction #{params[:transaction_id]} not found" }, :status => 404
+      render :json => { :status => 'error', :message => "Transaction #{params[:transaction_id]} not found" }.as_json(:allow_nan => true), :status => 404
     end
   rescue => error
-    render :json => { :status => 'error', :message => error.message }, :status => 500
+    render :json => { :status => 'error', :message => "#{error.message}\n#{error.backtrace.join("\n")}" }.as_json(:allow_nan => true), :status => 500
   end
 
   # Resume.request (transaction ID)
@@ -74,10 +74,10 @@ def resume
     if transaction
       render json: transaction.id
     else
-      render :json => { :status => 'error', :message => "Transaction #{params[:transaction_id]} not found" }, :status => 404
+      render :json => { :status => 'error', :message => "Transaction #{params[:transaction_id]} not found" }.as_json(:allow_nan => true), :status => 404
     end
   rescue => error
-    render :json => { :status => 'error', :message => error.message }, :status => 500
+    render :json => { :status => 'error', :message => "#{error.message}\n#{error.backtrace.join("\n")}" }.as_json(:allow_nan => true), :status => 500
   end
 
   # Report.request (transaction ID)
@@ -88,10 +88,10 @@ def report
     if transaction
       render json: transaction.id
     else
-      render :json => { :status => 'error', :message => "Transaction #{params[:transaction_id]} not found" }, :status => 404
+      render :json => { :status => 'error', :message => "Transaction #{params[:transaction_id]} not found" }.as_json(:allow_nan => true), :status => 404
     end
   rescue => error
-    render :json => { :status => 'error', :message => error.message }, :status => 500
+    render :json => { :status => 'error', :message => "#{error.message}\n#{error.backtrace.join("\n")}" }.as_json(:allow_nan => true), :status => 500
   end
 
   def directory_listing
@@ -100,17 +100,17 @@ def directory_listing
     transaction = $cfdp_user.start_directory_listing(params)
     render json: transaction.id
   rescue ActionController::ParameterMissing => error
-    render :json => { :status => 'error', :message => error.message }, :status => 400
+    render :json => { :status => 'error', :message => error.message }.as_json(:allow_nan => true), :status => 400
   rescue => error
-    render :json => { :status => 'error', :message => error.message }, :status => 500
+    render :json => { :status => 'error', :message => "#{error.message}\n#{error.backtrace.join("\n")}" }.as_json(:allow_nan => true), :status => 500
   end
 
   def subscribe
     return unless check_authorization()
     result = CfdpTopic.subscribe_indications
-    render json: result
+    render json: result.as_json(:allow_nan => true)
   rescue => error
-    render :json => { :status => 'error', :message => error.message }, :status => 500
+    render :json => { :status => 'error', :message => "#{error.message}\n#{error.backtrace.join("\n")}" }.as_json(:allow_nan => true), :status => 500
   end
 
   # Transaction.indication (transaction ID)
@@ -149,9 +149,9 @@ def subscribe
   def indications
     return unless check_authorization()
     result = CfdpTopic.read_indications(transaction_id: params[:transaction_id], continuation: params[:continuation], limit: params[:limit])
-    render json: result
+    render json: result.as_json(:allow_nan => true)
   rescue => error
-    render :json => { :status => 'error', :message => error.message }, :status => 500
+    render :json => { :status => 'error', :message => "#{error.message}\n#{error.backtrace.join("\n")}" }.as_json(:allow_nan => true), :status => 500
   end
 
   def transactions
@@ -166,9 +166,9 @@ def transactions
       end
     end
     result = result.sort {|a, b| a.id <=> b.id}
-    render json: result
+    render json: result.as_json(:allow_nan => true)
   rescue => error
-    render :json => { :status => 'error', :message => error.message }, :status => 500
+    render :json => { :status => 'error', :message => "#{error.message}\n#{error.backtrace.join("\n")}" }.as_json(:allow_nan => true), :status => 500
   end
 
   # private
@@ -179,14 +179,14 @@ def check_authorization
 
     if params[:remote_entity_id]
       if params[:remote_entity_id].to_i.to_s != params[:remote_entity_id].to_s
-        render :json => { :status => 'error', :message => "remote_entity_id must be numeric" }, :status => 400
+        render :json => { :status => 'error', :message => "remote_entity_id must be numeric" }.as_json(:allow_nan => true), :status => 400
         return false
       end
       cmd_entity_id = Integer(params[:remote_entity_id])
       cmd_entity = CfdpMib.entity(cmd_entity_id)
     elsif params[:destination_entity_id]
       if params[:destination_entity_id].to_i.to_s != params[:destination_entity_id].to_s
-        render :json => { :status => 'error', :message => "destination_entity_id must be numeric" }, :status => 400
+        render :json => { :status => 'error', :message => "destination_entity_id must be numeric" }.as_json(:allow_nan => true), :status => 400
         return
       end
       cmd_entity_id = Integer(params[:destination_entity_id])
@@ -207,11 +207,11 @@ def check_authorization
         # Caller must be able to send this command
         return false unless authorization('cmd', target_name: target_name, packet_name: packet_name)
       else
-        render :json => { :status => 'error', :message => "info not configured for entity: #{cmd_entity_id}" }, :status => 400
+        render :json => { :status => 'error', :message => "info not configured for entity: #{cmd_entity_id}" }.as_json(:allow_nan => true), :status => 400
         return false
       end
     else
-      render :json => { :status => 'error', :message => "Unknown entity: #{cmd_entity_id}" }, :status => 400
+      render :json => { :status => 'error', :message => "Unknown entity: #{cmd_entity_id}" }.as_json(:allow_nan => true), :status => 400
       return false
     end
 

microservices/CFDP/app/models/cfdp_pdu.rb

@@ -30,6 +30,9 @@
 require 'cfdp_pdu/cfdp_pdu_user_ops'
 
 class CfdpPdu < OpenC3::Packet
+  DIRECTIVE_CODE_BYTE_SIZE = 1
+  CRC_BYTE_SIZE = 2
+
   def initialize(crcs_required:)
     super()
     append_item("VERSION", 3, :UINT)

microservices/CFDP/lib/cfdp_pdu/cfdp_pdu_ack.rb

@@ -35,9 +35,10 @@ def self.build_ack_pdu(
     transaction_status:)
 
     pdu = build_initial_pdu(type: "FILE_DIRECTIVE", destination_entity: destination_entity, transmission_mode: transmission_mode, file_size: 0, segmentation_control: segmentation_control)
-    pdu_header_part_1_length = pdu.length # Measured here before writing variable data
+    pdu_header_part_1_length = pdu.length # Measured here before writing variable data - Includes CRC if present
+    pdu_header_part_1_length -= CRC_BYTE_SIZE if destination_entity['crcs_required'] # PDU_DATA_LENGTH field should contain CRC length
     pdu_header = pdu.build_variable_header(source_entity_id: source_entity['id'], transaction_seq_num: transaction_seq_num, destination_entity_id: destination_entity['id'], directive_code: "ACK")
-    pdu_header_part_2_length = pdu_header.length
+    pdu_header_part_2_length = pdu_header.length - DIRECTIVE_CODE_BYTE_SIZE # Minus 1 = Directive code is part of data per 5.2.1.1
     if ack_directive_code == "FINISHED" or ack_directive_code == 5
       pdu.write("DIRECTION", "TOWARD_FILE_RECEIVER")
     else

microservices/CFDP/lib/cfdp_pdu/cfdp_pdu_eof.rb

@@ -39,9 +39,10 @@ def self.build_eof_pdu(
     canceling_entity_id: nil)
 
     pdu = build_initial_pdu(type: "FILE_DIRECTIVE", destination_entity: destination_entity, transmission_mode: transmission_mode, file_size: file_size, segmentation_control: segmentation_control)
-    pdu_header_part_1_length = pdu.length # Measured here before writing variable data
+    pdu_header_part_1_length = pdu.length # Measured here before writing variable data - Includes CRC if present
+    pdu_header_part_1_length -= CRC_BYTE_SIZE if destination_entity['crcs_required'] # PDU_DATA_LENGTH field should contain CRC length
     pdu_header = pdu.build_variable_header(source_entity_id: source_entity['id'], transaction_seq_num: transaction_seq_num, destination_entity_id: destination_entity['id'], directive_code: "EOF")
-    pdu_header_part_2_length = pdu_header.length
+    pdu_header_part_2_length = pdu_header.length - DIRECTIVE_CODE_BYTE_SIZE # Minus 1 = Directive code is part of data per 5.2.1.1
     pdu_contents = pdu.build_eof_pdu_contents(condition_code: condition_code, file_checksum: file_checksum, file_size: file_size, canceling_entity_id: canceling_entity_id)
     pdu.write("VARIABLE_DATA", pdu_header + pdu_contents)
     pdu.write("PDU_DATA_LENGTH", pdu.length - pdu_header_part_1_length - pdu_header_part_2_length)

microservices/CFDP/lib/cfdp_pdu/cfdp_pdu_file_data.rb

@@ -43,7 +43,8 @@ def self.build_file_data_pdu(
     segment_metadata: nil)
 
     pdu = build_initial_pdu(type: "FILE_DATA", destination_entity: destination_entity, file_size: file_size, segmentation_control: segmentation_control, transmission_mode: transmission_mode)
-    pdu_header_part_1_length = pdu.length # Measured here before writing variable data
+    pdu_header_part_1_length = pdu.length # Measured here before writing variable data - Includes CRC if present
+    pdu_header_part_1_length -= CRC_BYTE_SIZE if destination_entity['crcs_required'] # PDU_DATA_LENGTH field should contain CRC length
     pdu_header = pdu.build_variable_header(source_entity_id: source_entity['id'], transaction_seq_num: transaction_seq_num, destination_entity_id: destination_entity['id'])
     pdu_header_part_2_length = pdu_header.length
     pdu_contents = pdu.build_file_data_pdu_contents(offset: offset, file_data: file_data, record_continuation_state: record_continuation_state, segment_metadata: segment_metadata)

microservices/CFDP/lib/cfdp_pdu/cfdp_pdu_finished.rb

@@ -41,9 +41,10 @@ def self.build_finished_pdu(
 
     pdu = build_initial_pdu(type: "FILE_DIRECTIVE", destination_entity: destination_entity, transmission_mode: transmission_mode, file_size: 0, segmentation_control: segmentation_control)
     pdu.write("DIRECTION", "TOWARD_FILE_SENDER")
-    pdu_header_part_1_length = pdu.length # Measured here before writing variable data
+    pdu_header_part_1_length = pdu.length # Measured here before writing variable data - Includes CRC if present
+    pdu_header_part_1_length -= CRC_BYTE_SIZE if destination_entity['crcs_required'] # PDU_DATA_LENGTH field should contain CRC length
     pdu_header = pdu.build_variable_header(source_entity_id: source_entity['id'], transaction_seq_num: transaction_seq_num, destination_entity_id: destination_entity['id'], directive_code: "FINISHED")
-    pdu_header_part_2_length = pdu_header.length
+    pdu_header_part_2_length = pdu_header.length - DIRECTIVE_CODE_BYTE_SIZE # Minus 1 = Directive code is part of data per 5.2.1.1
     pdu_contents = pdu.build_finished_pdu_contents(condition_code: condition_code, delivery_code: delivery_code, file_status: file_status, filestore_responses: filestore_responses, fault_location_entity_id: fault_location_entity_id)
     pdu.write("VARIABLE_DATA", pdu_header + pdu_contents)
     pdu.write("PDU_DATA_LENGTH", pdu.length - pdu_header_part_1_length - pdu_header_part_2_length)

microservices/CFDP/lib/cfdp_pdu/cfdp_pdu_keep_alive.rb

@@ -32,9 +32,10 @@ def self.build_keep_alive_pdu(
 
     pdu = build_initial_pdu(type: "FILE_DIRECTIVE", destination_entity: destination_entity, transmission_mode: transmission_mode, file_size: file_size, segmentation_control: segmentation_control)
     pdu.write("DIRECTION", "TOWARD_FILE_SENDER")
-    pdu_header_part_1_length = pdu.length # Measured here before writing variable data
+    pdu_header_part_1_length = pdu.length # Measured here before writing variable data - Includes CRC if present
+    pdu_header_part_1_length -= CRC_BYTE_SIZE if destination_entity['crcs_required'] # PDU_DATA_LENGTH field should contain CRC length
     pdu_header = pdu.build_variable_header(source_entity_id: source_entity['id'], transaction_seq_num: transaction_seq_num, destination_entity_id: destination_entity['id'], directive_code: "KEEP_ALIVE")
-    pdu_header_part_2_length = pdu_header.length
+    pdu_header_part_2_length = pdu_header.length - DIRECTIVE_CODE_BYTE_SIZE # Minus 1 = Directive code is part of data per 5.2.1.1
     pdu_contents = pdu.build_keep_alive_pdu_contents(progress: progress)
     pdu.write("VARIABLE_DATA", pdu_header + pdu_contents)
     pdu.write("PDU_DATA_LENGTH", pdu.length - pdu_header_part_1_length - pdu_header_part_2_length)

microservices/CFDP/lib/cfdp_pdu/cfdp_pdu_metadata.rb

@@ -50,9 +50,10 @@ def self.build_metadata_pdu(
     options: [])
 
     pdu = build_initial_pdu(type: "FILE_DIRECTIVE", destination_entity: destination_entity, transmission_mode: transmission_mode, file_size: file_size, segmentation_control: segmentation_control)
-    pdu_header_part_1_length = pdu.length # Measured here before writing variable data
+    pdu_header_part_1_length = pdu.length # Measured here before writing variable data - Includes CRC if present
+    pdu_header_part_1_length -= CRC_BYTE_SIZE if destination_entity['crcs_required'] # PDU_DATA_LENGTH field should contain CRC length
     pdu_header = pdu.build_variable_header(source_entity_id: source_entity['id'], transaction_seq_num: transaction_seq_num, destination_entity_id: destination_entity['id'], directive_code: "METADATA")
-    pdu_header_part_2_length = pdu_header.length
+    pdu_header_part_2_length = pdu_header.length - DIRECTIVE_CODE_BYTE_SIZE # Minus 1 = Directive code is part of data per 5.2.1.1
     if checksum_type_implemented(destination_entity['default_checksum_type'])
       checksum_type = destination_entity['default_checksum_type']
     else

microservices/CFDP/lib/cfdp_pdu/cfdp_pdu_nak.rb

@@ -44,9 +44,10 @@ def self.build_nak_pdu(
 
     pdu = build_initial_pdu(type: "FILE_DIRECTIVE", destination_entity: destination_entity, transmission_mode: transmission_mode, file_size: file_size, segmentation_control: segmentation_control)
     pdu.write("DIRECTION", "TOWARD_FILE_SENDER")
-    pdu_header_part_1_length = pdu.length # Measured here before writing variable data
+    pdu_header_part_1_length = pdu.length # Measured here before writing variable data - Includes CRC if present
+    pdu_header_part_1_length -= CRC_BYTE_SIZE if destination_entity['crcs_required'] # PDU_DATA_LENGTH field should contain CRC length
     pdu_header = pdu.build_variable_header(source_entity_id: source_entity['id'], transaction_seq_num: transaction_seq_num, destination_entity_id: destination_entity['id'], directive_code: "NAK")
-    pdu_header_part_2_length = pdu_header.length
+    pdu_header_part_2_length = pdu_header.length - DIRECTIVE_CODE_BYTE_SIZE # Minus 1 = Directive code is part of data per 5.2.1.1
     pdu_contents = pdu.build_nak_pdu_contents(start_of_scope: start_of_scope, end_of_scope: end_of_scope, segment_requests: segment_requests)
     pdu.write("VARIABLE_DATA", pdu_header + pdu_contents)
     pdu.write("PDU_DATA_LENGTH", pdu.length - pdu_header_part_1_length - pdu_header_part_2_length)

microservices/CFDP/lib/cfdp_pdu/cfdp_pdu_prompt.rb

@@ -30,9 +30,10 @@ def self.build_prompt_pdu(
     response_required:)
 
     pdu = build_initial_pdu(type: "FILE_DIRECTIVE", destination_entity: destination_entity, transmission_mode: transmission_mode, file_size: 0, segmentation_control: segmentation_control)
-    pdu_header_part_1_length = pdu.length # Measured here before writing variable data
+    pdu_header_part_1_length = pdu.length # Measured here before writing variable data - Includes CRC if present
+    pdu_header_part_1_length -= CRC_BYTE_SIZE if destination_entity['crcs_required'] # PDU_DATA_LENGTH field should contain CRC length
     pdu_header = pdu.build_variable_header(source_entity_id: source_entity['id'], transaction_seq_num: transaction_seq_num, destination_entity_id: destination_entity['id'], directive_code: "PROMPT")
-    pdu_header_part_2_length = pdu_header.length
+    pdu_header_part_2_length = pdu_header.length - DIRECTIVE_CODE_BYTE_SIZE # Minus 1 = Directive code is part of data per 5.2.1.1
     pdu_contents = pdu.build_prompt_pdu_contents(response_required: response_required)
     pdu.write("VARIABLE_DATA", pdu_header + pdu_contents)
     pdu.write("PDU_DATA_LENGTH", pdu.length - pdu_header_part_1_length - pdu_header_part_2_length)

microservices/CFDP/spec/models/cfdp_pdu_ack_spec.rb

@@ -53,6 +53,7 @@
 
       # By default the first 7 bytes are the header
       # This assumes 1 byte per entity ID and sequence number
+      expect(buffer[1..2].unpack('n')[0]).to eql 5 # PDU_DATA_LENGTH - Directive Code plus Ack Data plus CRC
 
       # Directive Code
       expect(buffer[7].unpack('C')[0]).to eql 6 # ACK per Table 5-4

microservices/CFDP/spec/models/cfdp_pdu_eof_spec.rb

@@ -55,6 +55,7 @@
 
       # By default the first 7 bytes are the header
       # This assumes 1 byte per entity ID and sequence number
+      expect(buffer[1..2].unpack('n')[0]).to eql 12 # PDU_DATA_LENGTH - Directive Code plus Data plus CRC
 
       # Directive Code
       expect(buffer[7].unpack('C')[0]).to eql 4 # EOF per Table 5-4

microservices/CFDP/spec/models/cfdp_pdu_file_data_spec.rb

@@ -56,6 +56,7 @@
 
       # By default the first 7 bytes are the header
       # This assumes 1 byte per entity ID and sequence number
+      expect(buffer[1..2].unpack('n')[0]).to eql 8 # PDU_DATA_LENGTH - Directive Code plus Data plus CRC
 
       # PDU Type
       expect((buffer[0].unpack('C')[0] >> 4) & 0x1).to eql 1 # File Data per Table 5-1

microservices/CFDP/spec/models/cfdp_pdu_finished_spec.rb

@@ -56,6 +56,7 @@
 
       # By default the first 7 bytes are the header
       # This assumes 1 byte per entity ID and sequence number
+      expect(buffer[1..2].unpack('n')[0]).to eql 4 # PDU_DATA_LENGTH - Directive Code plus Data plus CRC
 
       # Directive Code
       expect(buffer[7].unpack('C')[0]).to eql 5 # Finished per Table 5-4

microservices/CFDP/spec/models/cfdp_pdu_keep_alive_spec.rb

@@ -53,6 +53,7 @@
 
       # By default the first 7 bytes are the header
       # This assumes 1 byte per entity ID and sequence number
+      expect(buffer[1..2].unpack('n')[0]).to eql 7 # PDU_DATA_LENGTH - Directive Code plus Data plus CRC
 
       # Directive Code
       expect(buffer[7].unpack('C')[0]).to eql 0x0C # Keep Alive per Table 5-4