Simplify Servo.cold_reset() to be more useful and reliable.
The cold_reset() method would set the 'cold_reset' signal 'on',
but never set it back to 'off'. This was unhelpful at best, and
outright wrong at worst. This change fixes the behavior to Do The
Right Thing.
The change also refactors servo initialization methods, and
adjusts the one caller of cold_reset() to use the new API. Along
the way, it fixes an outstanding TODO, cited below.
CQ-DEPEND=Ief5581e1e453541752ecbea221c0a926c94a364b
BUG=chromium-os:23626
TEST=selected FAFT tests
Change-Id: I018173af9b8f5dda7966644948654b85bf5c01f7
Reviewed-on: https://gerrit.chromium.org/gerrit/27951
Tested-by: Richard Barnette <jrbarnette@chromium.org>
Reviewed-by: Tom Wai-Hong Tam <waihong@chromium.org>
Tested-by: Tom Wai-Hong Tam <waihong@chromium.org>
Commit-Ready: Richard Barnette <jrbarnette@chromium.org>
diff --git a/server/cros/servo.py b/server/cros/servo.py
index ef85fb3..5e0e91d 100644
--- a/server/cros/servo.py
+++ b/server/cros/servo.py
@@ -42,6 +42,11 @@
RECOVERY_BOOT_DELAY = 10
RECOVERY_INSTALL_DELAY = 540
+ # Time required for the EC to be working after cold reset.
+ # Five seconds is at least twice as big as necessary for Alex,
+ # and is presumably good enough for all future systems.
+ _EC_RESET_DELAY = 5.0
+
# Servo-specific delays.
MAX_SERVO_STARTUP_DELAY = 10
SERVO_SEND_SIGNAL_DELAY = 0.5
@@ -88,28 +93,41 @@
return None
- def __init__(self, servo_host='localhost', servo_port=9999,
- cold_reset=False):
+ def __init__(self, servo_host='localhost', servo_port=9999):
"""Sets up the servo communication infrastructure.
@param servo_host Name of the host where the servod process
is running.
@param servo_port Port the servod process is listening on.
- @param cold_reset If True, cold reset device and boot during init,
- otherwise perform init with device running.
"""
self._server = None
-
- self._do_cold_reset = cold_reset
self._connect_servod(servo_host, servo_port)
- def initialize_dut(self):
- """Initializes a dut for testing purposes."""
- if self._do_cold_reset:
- self._init_seq_cold_reset_devmode()
- else:
- self._init_seq()
+ def initialize_dut(self, cold_reset=False):
+ """Initializes a dut for testing purposes.
+
+ This sets various servo signals back to default values
+ appropriate for the target board. By default, if the DUT
+ is already on, it stays on. If the DUT is powered off
+ before initialization, its state afterward is unspecified.
+
+ If cold reset is requested, the DUT is guaranteed to be off
+ at the end of initialization, regardless of its initial
+ state.
+
+ Rationale: Basic initialization of servo sets the lid open,
+ when there is a lid. This operation won't affect powered on
+ units; however, setting the lid open may power on a unit
+ that's off, depending on factors outside the scope of this
+ function.
+
+ @param cold_reset If True, cold reset the device after
+ initialization.
+ """
+ self._server.hwinit()
+ if cold_reset:
+ self.cold_reset()
def power_long_press(self):
@@ -292,11 +310,15 @@
def cold_reset(self):
"""Perform a cold reset of the EC.
- Has the side effect of shutting off the device. Device is guaranteed
- to be off at the end of this call.
+ This has the side effect of shutting off the device. The
+ device is guaranteed to be off at the end of this call.
"""
+ # After the reset, give the EC the time it needs to
+ # re-initialize.
self.set('cold_reset', 'on')
time.sleep(Servo.SERVO_SEND_SIGNAL_DELAY)
+ self.set('cold_reset', 'off')
+ time.sleep(self._EC_RESET_DELAY)
def warm_reset(self):
@@ -481,31 +503,6 @@
raise
- def _init_seq_cold_reset_devmode(self):
- """Cold reset, init device, and boot in dev-mode."""
- self.cold_reset()
- self._init_seq()
- self.set('dev_mode', 'on')
- self.boot_devmode()
-
-
- def _init_seq(self):
- """Initiate starting state for servo."""
- # TODO(tbroch) This is only a servo V1 control. Need to add ability in
- # servod to easily identify version so I can make this conditional not
- # try and fail quietly
- try:
- self.set('tx_dir', 'input')
- except:
- logging.warning("Failed to set tx_dir. This is ok if not servo V1")
-
-
- # TODO(tbroch) Investigate method to determine DUT's type so we can
- # conditionally set lid if applicable
- self.set_nocheck('lid_open', 'yes')
- self.set('rec_mode', 'off')
-
-
def _connect_servod(self, servo_host, servo_port):
"""Connect to the Servod process with XMLRPC.