Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Sprinkler] Initialize timers early to avoid crash #5499

Merged
merged 6 commits into from Oct 9, 2023

Conversation

hostcc
Copy link
Contributor

@hostcc hostcc commented Oct 8, 2023

What does this implement/fix?

Attempting to set standby_switch of a controller to ON at early boot stages results in the crash below. Such flow could be introduced by a switch that sets controller(s) to standby (i.e. a global disable switch).

The issue is resolved by adding non-default constructor to Sprinkler class that initializes timers (valve selection and state machine ones) early, hence activating standby (and shutdown action it invokes internally) is now a safe operation.

(gdb) bt
#0  isr_hardfault () at /home/earle/Arduino/hardware/pico/rp2040/pico-sdk/src/rp2_common/pico_standard_link/crt0.S:98
#1  <signal handler called>
#2  0x47701c18 in ?? ()
#3  0x10016206 in std::function<void ()>::function(std::function<void ()> const&) ()
#4  0x10016234 in esphome::sprinkler::Sprinkler::start_timer_(esphome::sprinkler::SprinklerTimerIndex) ()
#5  0x10016292 in esphome::sprinkler::Sprinkler::fsm_transition_to_shutdown_() ()
#6  0x100162d0 in esphome::sprinkler::Sprinkler::shutdown(bool) ()
#7  0x100162f6 in esphome::sprinkler::ShutdownAction<>::play() ()
#8  0x100149da in esphome::Action<>::play_complex() ()
#9  0x10014caa in esphome::Trigger<>::trigger() [clone .isra.0] ()
#10 0x10014cd8 in esphome::sprinkler::SprinklerControllerSwitch::write_state(bool) ()
#11 0x100174f2 in esphome::switch_::Switch::turn_on() ()
#12 0x1001d9c6 in std::_Function_handler<void (), setup::{lambda()#48}>::_M_invoke(std::_Any_data const&) ()
#13 0x1001d80c in esphome::LambdaAction<>::play() ()
#14 0x100149da in esphome::Action<>::play_complex() ()
#15 0x10017cae in esphome::Trigger<>::trigger() [clone .isra.0] ()
#16 0x10017cdc in esphome::template_::TemplateSwitch::write_state(bool) ()
#17 0x100174f2 in esphome::switch_::Switch::turn_on() ()
#18 0x10017c0e in esphome::template_::TemplateSwitch::setup() ()
#19 0x1001bdd0 in esphome::Component::call_setup() ()
#20 0x1001beb0 in esphome::Component::call() ()
#21 0x1001bbfe in esphome::Application::setup() ()
#22 0x100252d4 in setup ()
#23 0x00000000 in ?? ()

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other

Related issue or feature (if applicable): N/A

Pull request in esphome-docs with documentation (if applicable): N/A

Test Environment

  • ESP32
  • ESP32 IDF
  • ESP8266
  • RP2040

Example entry for config.yaml:

# Example config.yaml
sprinkler:
  - id: sprinkler_ctrlr
    main_switch: "Sprinklers"
    auto_advance_switch: "Sprinklers Auto Advance"
    standby_switch:
      name: "Sprinklers Pause"
      id: sprinkler_ctrlr_standby_switch
    valves:
      - valve_switch: "Front Lawn"
        enable_switch: "Enable Front Lawn"
        run_duration: 1800s
        valve_switch_id: lawn_sprinkler_valve_sw0

switch:
  - platform: template
    id: winter_mode
    name: "Winter mode"
    optimistic: true
    restore_mode: RESTORE_DEFAULT_OFF
    entity_category: config
    turn_on_action:
      - lambda: |-
          id(sprinkler_ctrlr).shutdown();
          id(sprinkler_ctrlr_standby_switch).turn_on();
    turn_off_action:
      # Added for demo purposes to trigger the scenario even at first boot upon flashing the firmware
      - lambda: |-
          id(sprinkler_ctrlr).shutdown();
          id(sprinkler_ctrlr_standby_switch).turn_on();

Checklist:

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:

Attempting to set `standby_switch` of a controller to ON at early boot
stages results in the crash below. Such flow could be introduced by a
switch that sets controller(s) to standby (i.e. a global disable
switch).

The issue is resolved by adding non-default constructor to `Sprinkler`
class that initialized timers (valve selection and state machine ones)
early, hence activating standby (and shutdown action it invokes
internally) is now a safe operation.

	(gdb) bt
	#0  isr_hardfault () at /home/earle/Arduino/hardware/pico/rp2040/pico-sdk/src/rp2_common/pico_standard_link/crt0.S:98
	esphome#1  <signal handler called>
	esphome#2  0x47701c18 in ?? ()
	esphome#3  0x10016206 in std::function<void ()>::function(std::function<void ()> const&) ()
	esphome#4  0x10016234 in esphome::sprinkler::Sprinkler::start_timer_(esphome::sprinkler::SprinklerTimerIndex) ()
	esphome#5  0x10016292 in esphome::sprinkler::Sprinkler::fsm_transition_to_shutdown_() ()
	esphome#6  0x100162d0 in esphome::sprinkler::Sprinkler::shutdown(bool) ()
	esphome#7  0x100162f6 in esphome::sprinkler::ShutdownAction<>::play() ()
	esphome#8  0x100149da in esphome::Action<>::play_complex() ()
	esphome#9  0x10014caa in esphome::Trigger<>::trigger() [clone .isra.0] ()
	esphome#10 0x10014cd8 in esphome::sprinkler::SprinklerControllerSwitch::write_state(bool) ()
	esphome#11 0x100174f2 in esphome::switch_::Switch::turn_on() ()
	esphome#12 0x1001d9c6 in std::_Function_handler<void (), setup::{lambda()esphome#48}>::_M_invoke(std::_Any_data const&) ()
	esphome#13 0x1001d80c in esphome::LambdaAction<>::play() ()
	esphome#14 0x100149da in esphome::Action<>::play_complex() ()
	esphome#15 0x10017cae in esphome::Trigger<>::trigger() [clone .isra.0] ()
	esphome#16 0x10017cdc in esphome::template_::TemplateSwitch::write_state(bool) ()
	esphome#17 0x100174f2 in esphome::switch_::Switch::turn_on() ()
	esphome#18 0x10017c0e in esphome::template_::TemplateSwitch::setup() ()
	esphome#19 0x1001bdd0 in esphome::Component::call_setup() ()
	esphome#20 0x1001beb0 in esphome::Component::call() ()
	esphome#21 0x1001bbfe in esphome::Application::setup() ()
	esphome#22 0x100252d4 in setup ()
	esphome#23 0x00000000 in ?? ()
@hostcc hostcc marked this pull request as ready for review October 8, 2023 19:19
@hostcc hostcc requested a review from kbx81 as a code owner October 8, 2023 19:19
@probot-esphome
Copy link

probot-esphome bot commented Oct 8, 2023

Hey there @kbx81, mind taking a look at this pull request as it has been labeled with an integration (sprinkler) you are listed as a code owner for? Thanks!
(message by CodeOwnersMention)

Copy link
Member

@kbx81 kbx81 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for finding and fixing this! Nice solution & bit of clean-up. 😄

@kbx81 kbx81 merged commit c65d78f into esphome:dev Oct 9, 2023
29 checks passed
@hostcc hostcc deleted the bugfix/sprinkler-timer-init-early branch October 9, 2023 03:46
@github-actions github-actions bot locked and limited conversation to collaborators Oct 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants