Multi variable bit size #1436

ryanmelt · 2024-07-25T14:31:53Z

closes #78

codecov · 2024-07-25T14:34:00Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.77%. Comparing base (6b9608b) to head (3840c30).
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1436      +/-   ##
==========================================
+ Coverage   75.65%   75.77%   +0.12%     
==========================================
  Files         603      605       +2     
  Lines       44832    45025     +193     
  Branches      787      787              
==========================================
+ Hits        33918    34119     +201     
+ Misses      10827    10818       -9     
- Partials       87       88       +1

Flag	Coverage Δ
frontend	`54.86% <ø> (-0.04%)`	⬇️
python	`84.00% <ø> (ø)`
ruby-api	`48.97% <ø> (ø)`
ruby-backend	`81.07% <ø> (+0.21%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

jmthomas

I still need to run locally and see about the coverage and additional testing

jmthomas · 2024-07-25T16:18:23Z

openc3/data/config/item_modifiers.yaml

-      required: true
-      description: The key to access this item
-      values: .+
-  example: KEY $.book.title


Why was this removed? You still have an example in access_tlm.txt of KEY on an ITEM.

It was just remove from item_modifiers because it was a dup (I think). It is also in param_item_modifiers.

Right and that's used by both ... not sure why it didn't get duplicated in the output though

jmthomas · 2024-07-25T16:20:56Z

openc3/lib/openc3/accessors/binary_accessor.rb

+            item.bit_size = 30
+          else
+            item.bit_size = 62
+          end


Why are they all 2 bits short?

Its the QUIC variable size integer format. See: https://datatracker.ietf.org/doc/html/rfc9000#name-variable-length-integer-enc

That's above and beyond ... I thought you were just going to do strings, lol

jmthomas · 2024-07-25T16:27:50Z

openc3/lib/openc3/accessors/binary_accessor.rb

+            # Skip items before this item and derived items and items with negative bit offsets
+            next
+          end
+          if sitem != item


The <=> operator in structure_item is based on bit_offset so you might be able to remove this check and just change the above check to sitem.bit_offset <= item.bit_offset

Maybe, but this code appears to work with my current testing.

jmthomas · 2024-07-25T16:32:20Z

openc3/lib/openc3/packets/structure.rb

+            # Minimum QUIC encoded integer
+            minimum_data_bits = 6
+          elsif item.variable_bit_size['length_value_bit_offset'] > 0
+            minimum_data_bits = item.variable_bit_size['length_value_bit_offset'] * item.variable_bit_size['length_bits_per_count']


This is effectively the STRING and BLOCK section?

Its STRING, BLOCK, or array

jmthomas · 2024-07-25T16:40:45Z

openc3/spec/accessors/binary_accessor_spec.rb

+        @packet.write("item2_length", 8) # Lengths must be initialized to zero equivalent
+        item2.variable_bit_size = {'length_item_name' => 'item2_length', 'length_value_bit_offset' => -64, 'length_bits_per_count' => 8}
+        expect(item2.bit_offset).to eql 48
+        expect(@packet.buffer).to eql "\x00\x00\x00\x04\x00\x08"


I'm not sure what's going on with this test. How did we get the 4 and 8? Negative offset is a positive value? What happens if you have a value offset of +32?

This test is handling the case where the value field includes itself in the count (so that has to be subtracted out)
A positive offset would be something like CCSDS where a value of 0 = 1 byte = 8 bits. Don't have a test for that yet.

jmthomas · 2024-07-25T19:09:04Z

openc3/lib/openc3/packets/packet_config.rb

+        parser.verify_num_parameters(0, 0, usage)
+        @current_packet.hidden = true
+        @current_packet.disabled = true
+        @current_packet.virtual = true


What is a VIRTUAL packet and how is it going to be used?

VIRTUAL packs aren't involved in identification. So it provides a way to declare embedded structures that can be used within packets. Effectively packets in packets.

jmthomas · 2024-07-25T19:37:52Z

openc3/lib/openc3/conversions/object_read_conversion.rb

+require 'openc3/conversions/conversion'
+
+module OpenC3
+  class ObjectReadConversion < Conversion


What is this for?

Used with virtual. This is a new conversion that references a packet structure, and then dumps the BLOCK data into that packet, and returns a hash of key/value pairs generated using the packet structure.

jmthomas · 2024-07-25T19:37:59Z

openc3/lib/openc3/conversions/object_write_conversion.rb

+require 'openc3/conversions/object_read_conversion'
+
+module OpenC3
+  class ObjectWriteConversion < Conversion


What is this for?

Used with virtual, to write a hash of items, into a packet, and then fill in a BLOCK item.

jmthomas · 2024-07-25T23:23:04Z

openc3/lib/openc3/accessors/binary_accessor.rb

+        elsif bytes < 0
+          # Remove extra bytes because we're adjusting smaller
+          buffer[item_offset + -bytes, -bytes] = ''
+        end


This is my new buffer adjustment logic

ryanmelt · 2024-07-26T02:21:08Z

openc3/lib/openc3/accessors/binary_accessor.rb

+              buffer[item_offset, original_length - item_offset]
+        elsif bytes < 0
+          # Remove extra bytes because we're adjusting smaller
+          buffer[item_offset + -bytes, -bytes] = ''


This doesn't seem right. Let's say we got 7 bytes smaller.
buffer[0 + 7, 7] = "" will destroy a bunch of real data.
Should it always be: buffer[item_offset + 1, -bytes] = '' ?

ryanmelt

Would like tests for the new conversion classes, and I think packet_config_spec should also include some new tests.

sonarqubecloud · 2024-07-29T04:12:58Z

Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

jmthomas · 2024-07-29T04:18:52Z

Would like tests for the new conversion classes, and I think packet_config_spec should also include some new tests.

Tests added and packet_config.rb is at 96.58%

ryanmelt · 2024-07-29T04:29:59Z

Looks good!

ryanmelt added 2 commits July 23, 2024 15:29

initial variable_bit_size

11622db

merge main

9c70e4d

ryanmelt requested a review from jmthomas July 25, 2024 14:31

ryanmelt requested a review from JL-Brothers July 25, 2024 14:48

jmthomas reviewed Jul 25, 2024

View reviewed changes

Regen docs

b278541

jmthomas reviewed Jul 25, 2024

View reviewed changes

jmthomas added 4 commits July 25, 2024 17:03

Fix variable sized integers

8187a08

Fix structure_item_spec

76d73d8

Merge branch 'main' into multi_variable_bit_size

ba3920b

Restore Dockerfile

ac063e7

jmthomas reviewed Jul 25, 2024

View reviewed changes

ryanmelt commented Jul 26, 2024

View reviewed changes

Fix binary_accessor and improve spec

0d98d77

jmthomas approved these changes Jul 26, 2024

View reviewed changes

ryanmelt commented Jul 26, 2024

View reviewed changes

Additional testing

3840c30

jmthomas merged commit 632f65c into main Jul 29, 2024
25 checks passed

jmthomas deleted the multi_variable_bit_size branch July 29, 2024 13:15

jmthomas mentioned this pull request Aug 15, 2024

Ingesting PCM Packets following IRIG Telemetry Standards #1482

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Multi variable bit size #1436

Multi variable bit size #1436

ryanmelt commented Jul 25, 2024

codecov bot commented Jul 25, 2024 •

edited

Loading

jmthomas left a comment

jmthomas Jul 25, 2024

ryanmelt Jul 25, 2024

jmthomas Jul 25, 2024

jmthomas Jul 25, 2024

ryanmelt Jul 25, 2024

jmthomas Jul 25, 2024

jmthomas Jul 25, 2024

ryanmelt Jul 25, 2024

jmthomas Jul 25, 2024

ryanmelt Jul 25, 2024

jmthomas Jul 25, 2024

ryanmelt Jul 25, 2024

jmthomas Jul 25, 2024

ryanmelt Jul 25, 2024

jmthomas Jul 25, 2024

ryanmelt Jul 25, 2024

jmthomas Jul 25, 2024

ryanmelt Jul 25, 2024

jmthomas Jul 25, 2024

ryanmelt Jul 26, 2024 •

edited

Loading

ryanmelt left a comment

sonarqubecloud bot commented Jul 29, 2024

jmthomas commented Jul 29, 2024

ryanmelt commented Jul 29, 2024

Multi variable bit size #1436

Multi variable bit size #1436

Conversation

ryanmelt commented Jul 25, 2024

codecov bot commented Jul 25, 2024 • edited Loading

Codecov Report

jmthomas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryanmelt Jul 26, 2024 • edited Loading

Choose a reason for hiding this comment

ryanmelt left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Jul 29, 2024

Quality Gate passed

jmthomas commented Jul 29, 2024

ryanmelt commented Jul 29, 2024

codecov bot commented Jul 25, 2024 •

edited

Loading

ryanmelt Jul 26, 2024 •

edited

Loading