From 402a6c4e9d2f7fa2ed50c5e1cf4b1b1b33b0c8b2 Mon Sep 17 00:00:00 2001 From: Gilad Shanan Date: Thu, 2 May 2024 21:47:58 -0500 Subject: [PATCH 1/4] Update hint message --- config/locales/en.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index 71836a6ec8..b54d6cbfbc 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1194,7 +1194,9 @@ en: instruct: ip_port: "Leave blank unless your device is on a non-standard port" outlet: "e.g. 1 or 2" - secondary_outlet: "turned on/off in sync with the outlet above (Synaccess only)" + secondary_outlet: | + This outlet will be turned on/off in sync with the outlet above (Synaccess only).
+ If you enter a value in this field, you must toggle the relay to activate. reservation: instruct: reserve_interval: "The minutes of an hour on which a reservation is allowed to begin (e.g. if 5 reservations can be scheduled every 5 minutes)" From 1cec09242d856e75588c38d25dbb49c09d443528 Mon Sep 17 00:00:00 2001 From: Gilad Shanan Date: Thu, 2 May 2024 21:48:10 -0500 Subject: [PATCH 2/4] Remove logging --- app/models/power_relay.rb | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/app/models/power_relay.rb b/app/models/power_relay.rb index b09bcafc84..12a4f647dd 100644 --- a/app/models/power_relay.rb +++ b/app/models/power_relay.rb @@ -36,7 +36,6 @@ def toggle(status) toggled_status = relay_connection.toggle(outlet, status) if secondary_outlet secondary_toggled_status = relay_connection.toggle(secondary_outlet, status) - handle_mismatch_status(status, toggled_status) if toggled_status != secondary_toggled_status end toggled_status end @@ -49,24 +48,11 @@ def query_status relay_status = relay_connection.status(outlet) if secondary_outlet secondary_outlet_status = relay_connection.status(secondary_outlet) - handle_mismatch_status if relay_status != secondary_outlet_status end relay_status end end - # Returns: - # string - an error - def handle_mismatch_status(requested_status=nil, primary_status=nil) - event = if requested_status.present? - "toggling relays (#{requested_status ? "on" : "off"})" - else - "querying status" - end - msg = "Outlet statuses don't match after #{event} for relay #{id} - outlet #{outlet} is (#{primary_status ? "on" : "off"}), outlet #{secondary_outlet} is (#{primary_status ? "off" : "on"})" - Rollbar.error(msg, relay: id) - end - def relay_connection raise NotImplementedError.new("Subclass must define") end From 6620d601844b63471cd38ff820c110c478aa1534 Mon Sep 17 00:00:00 2001 From: Gilad Shanan Date: Thu, 2 May 2024 21:51:15 -0500 Subject: [PATCH 3/4] Sync up secondary and primary outlets when the secondary outlet changes We can't use an AR callback here because we generate new relay records on update. See Product#replace_relay --- app/controllers/instrument_relays_controller.rb | 2 ++ app/models/power_relay.rb | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/app/controllers/instrument_relays_controller.rb b/app/controllers/instrument_relays_controller.rb index 028f4e38e8..d3d2e725b8 100644 --- a/app/controllers/instrument_relays_controller.rb +++ b/app/controllers/instrument_relays_controller.rb @@ -36,8 +36,10 @@ def create private def handle_relay(action_string) + previous_secondary_outlet = @product.relay.secondary_outlet @relay = @product.replace_relay(relay_params, params[:relay][:control_mechanism]) if @relay.valid? + @relay.try(:activate_secondary_outlet) if @relay.secondary_outlet != previous_secondary_outlet flash[:notice] = "Relay was successfully updated." redirect_to facility_instrument_relays_path(current_facility, @product) else diff --git a/app/models/power_relay.rb b/app/models/power_relay.rb index 12a4f647dd..291528c92b 100644 --- a/app/models/power_relay.rb +++ b/app/models/power_relay.rb @@ -41,6 +41,13 @@ def toggle(status) end end + # This method will toggle the secondary outlet to match the primary outlet. + # Useful to sync up the outlets whenever the secondary outlet changes. + def activate_secondary_outlet + primary_outlet_status = relay_connection.status(outlet) + relay_connection.toggle(secondary_outlet, primary_outlet_status) + end + # Returns: # boolean - The current on/off status of the outlet (and secondary outlet, if configured). def query_status From a90cd6778e9a4803fce07d67b7532deb16216418 Mon Sep 17 00:00:00 2001 From: Gilad Shanan Date: Mon, 6 May 2024 16:56:19 -0500 Subject: [PATCH 4/4] Sync up secondary relay after any change This keeps the logic simpler and should always be safe. Any change results in a new relay instance, so otherwise we'd have to do some safe navigation like: previous_secondary_outlet = @product.relay&.secondary_outlet Not a huge deal but it crossed the line for me of complexity vs value. --- app/controllers/instrument_relays_controller.rb | 3 +-- spec/system/admin/instrument_relay_tab_spec.rb | 11 ++++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/controllers/instrument_relays_controller.rb b/app/controllers/instrument_relays_controller.rb index d3d2e725b8..74a7d7914f 100644 --- a/app/controllers/instrument_relays_controller.rb +++ b/app/controllers/instrument_relays_controller.rb @@ -36,10 +36,9 @@ def create private def handle_relay(action_string) - previous_secondary_outlet = @product.relay.secondary_outlet @relay = @product.replace_relay(relay_params, params[:relay][:control_mechanism]) if @relay.valid? - @relay.try(:activate_secondary_outlet) if @relay.secondary_outlet != previous_secondary_outlet + @relay.try(:activate_secondary_outlet) flash[:notice] = "Relay was successfully updated." redirect_to facility_instrument_relays_path(current_facility, @product) else diff --git a/spec/system/admin/instrument_relay_tab_spec.rb b/spec/system/admin/instrument_relay_tab_spec.rb index 30fb07e6a0..da410a4413 100644 --- a/spec/system/admin/instrument_relay_tab_spec.rb +++ b/spec/system/admin/instrument_relay_tab_spec.rb @@ -7,6 +7,7 @@ let(:user) { FactoryBot.create(:user, :administrator) } before do + allow_any_instance_of(RelaySynaccessRevA).to receive(:activate_secondary_outlet).and_return(true) login_as user visit facility_instrument_relays_path(facility, instrument) end @@ -104,7 +105,7 @@ fill_in "relay_building_room_number", with: "1a" fill_in "relay_circuit_number", with: "1" fill_in "relay_ethernet_port_number", with: "2000" - + click_button "Save" instrument.reload expect(instrument.relay).to be_present @@ -136,7 +137,7 @@ click_button "Save" expect(page).to have_content("Outlet has already been taken") end - + context "both instruments have the same schedule" do let!(:instrument2) { create(:instrument, facility: facility, no_relay: true, schedule: instrument.schedule) } let!(:existing_relay) { create(:relay_syna, instrument: instrument2) } @@ -199,7 +200,7 @@ context "switching relay types" do let(:instrument) { FactoryBot.create(:setup_instrument, facility: facility, relay: build(:relay)) } - + before do click_link "Edit" end @@ -213,7 +214,7 @@ expect(instrument.relay).to be_a(RelayDummy) end - + end context "from relay to reservation only" do @@ -226,7 +227,7 @@ expect(instrument.relay).not_to be_present end - + end context "from reservation only to timer" do