Merge pull request #1609 from OpenC3/item_limits

Disabling limits sets enabled to false
OpenC3 · Oct 12, 2024 · 58cd6aa · 58cd6aa
2 parents de8398a + 01f1d44
commit 58cd6aa
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 11 deletions.
diff --git a/openc3/lib/openc3/api/limits_api.rb b/openc3/lib/openc3/api/limits_api.rb
@@ -164,7 +164,7 @@ def disable_limits(*args, manual: false, scope: $openc3_scope, token: $openc3_to
       found_item = nil
       packet['items'].each do |item|
         if item['name'] == item_name
-          item['limits'].delete('enabled')
+          item['limits']['enabled'] = false
           found_item = item
           break
         end
@@ -224,7 +224,7 @@ def set_limits(target_name, packet_name, item_name, red_low, yellow_low, yellow_
             if enabled
               item['limits']['enabled'] = true
             else
-              item['limits'].delete('enabled')
+              item['limits']['enabled'] = false
             end
             limits = {}
             limits['red_low'] = red_low
@@ -348,7 +348,7 @@ def _limits_group(group_name, action:, manual:, scope:, token:)
               message = "Enabling Limits for '#{target_name} #{packet_name} #{item_name}'"
             elsif action == :disable
               enabled = false
-              item['limits'].delete('enabled')
+              item['limits']['enabled'] = false
               message = "Disabling Limits for '#{target_name} #{packet_name} #{item_name}'"
             end
             Logger.info(message, scope: scope)

diff --git a/openc3/lib/openc3/packets/packet_item.rb b/openc3/lib/openc3/packets/packet_item.rb
@@ -491,7 +491,11 @@ def as_json(*a)
 
       if self.limits
         config['limits'] ||= {}
-        config['limits']['enabled'] = true if self.limits.enabled
+        if self.limits.enabled
+          config['limits']['enabled'] = true
+        else
+          config['limits']['enabled'] = false
+        end
         if self.limits.values
           config['limits'] ||= {}
           config['limits']['persistence_setting'] = self.limits.persistence_setting

diff --git a/openc3/spec/api/limits_api_spec.rb b/openc3/spec/api/limits_api_spec.rb
@@ -39,17 +39,14 @@ class ApiTest
     before(:each) do
       @redis = mock_redis()
       setup_system()
+      local_s3()
 
       %w(INST SYSTEM).each do |target|
         model = TargetModel.new(folder_name: target, name: target, scope: "DEFAULT")
         model.create
         model.update_store(System.new([target], File.join(SPEC_DIR, 'install', 'config', 'targets')))
       end
 
-      # Mock out some stuff in Microservice initialize()
-      dbl = double("AwsS3Client").as_null_object
-      allow(Aws::S3::Client).to receive(:new).and_return(dbl)
-      allow(Zip::File).to receive(:open).and_return(true)
       allow_any_instance_of(OpenC3::Interface).to receive(:connected?).and_return(true)
       @im_shutdown = false
       allow_any_instance_of(OpenC3::Interface).to receive(:read_interface) { sleep(0.01) until @im_shutdown }
@@ -127,14 +124,14 @@ class ApiTest
         @api.set_limits("INST", "HEALTH_STATUS", "TEMP1", 0.0, 1.0, 4.0, 5.0, 2.0, 3.0, 'DEFAULT', 10, false)
         item = @api.get_item("INST", "HEALTH_STATUS", "TEMP1")
         expect(item['limits']['persistence_setting']).to eql(10)
-        expect(item['limits']['enabled']).to be_nil
+        expect(item['limits']['enabled']).to be false
         expect(item['limits']['DEFAULT']).to eql({ 'red_low' => 0.0, 'yellow_low' => 1.0, 'yellow_high' => 4.0,
                                                    'red_high' => 5.0, 'green_low' => 2.0, 'green_high' => 3.0 })
         # Verify it also works with symbols for the set
         @api.set_limits("INST", "HEALTH_STATUS", "TEMP1", 1.0, 2.0, 5.0, 6.0, 3.0, 4.0, :DEFAULT, 10, false)
         item = @api.get_item("INST", "HEALTH_STATUS", "TEMP1")
         expect(item['limits']['persistence_setting']).to eql(10)
-        expect(item['limits']['enabled']).to be_nil
+        expect(item['limits']['enabled']).to be false
         expect(item['limits']['DEFAULT']).to eql({ 'red_low' => 1.0, 'yellow_low' => 2.0, 'yellow_high' => 5.0,
                                                   'red_high' => 6.0, 'green_low' => 3.0, 'green_high' => 4.0 })
       end
@@ -391,9 +388,15 @@ class ApiTest
 
       it "disables limits for an item" do
         expect(@api.limits_enabled?("INST", "HEALTH_STATUS", "TEMP1")).to be true
+        item = @api.get_item("INST", "HEALTH_STATUS", "TEMP1")
+        expect(item['limits']['enabled']).to be true
         @api.disable_limits("INST", "HEALTH_STATUS", "TEMP1")
         expect(@api.limits_enabled?("INST", "HEALTH_STATUS", "TEMP1")).to be false
+        item = @api.get_item("INST", "HEALTH_STATUS", "TEMP1")
+        expect(item['limits']['enabled']).to be false
         @api.enable_limits("INST", "HEALTH_STATUS", "TEMP1")
+        item = @api.get_item("INST", "HEALTH_STATUS", "TEMP1")
+        expect(item['limits']['enabled']).to be true
       end
     end
   end

diff --git a/openc3/spec/packets/packet_item_spec.rb b/openc3/spec/packets/packet_item_spec.rb
@@ -479,7 +479,7 @@ module OpenC3
         expect(hash["maximum"]).to eql 100
         expect(hash["required"]).to be true
         expect(hash["limits"]).to_not be_nil
-        expect(hash["limits"]["enabled"]).to be_nil # Missing if false
+        expect(hash["limits"]["enabled"]).to be false
         # State is actually stored in Redis so it doesn't make sense to return via PacketItemLimits
         expect(hash["limits"]["state"]).to be_nil
         expect(hash["limits"]["response"]).to match("LimitsResponse")