New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 840961 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocked on:
issue 841097



Sign in to add a comment

dut-control: ec_uart_parity: cannot marshall None

Project Member Reported by briannorris@chromium.org, May 8 2018

Issue description

On Scarlet, with ToT hdctools:

$ dut-control --port=9992 ec_uart_parity
Problem with ['ec_uart_parity'] :: cannot marshal None unless allow_none is enabled

I'm quite sure this is a recent regression.
 
Cc: puthik@chromium.org coconutruben@chromium.org tbroch@chromium.org
Previously, this returned:

ec_uart_parity:None

Bisect tells me:

2b8661fb99c9a266429cae324c744db524d4211f is the first bad commit
commit 2b8661fb99c9a266429cae324c744db524d4211f
Author: Ruben Rodriguez Buchillon <coconutruben@chromium.org>
Date:   Thu Apr 19 14:13:45 2018 +0800

    servo: allow return of all supported xml-rpc objects
    
    Right now before servo_server returns something it gets cast into a
    string.
    As we build more code that interfaces with servod instances it becomes
    more cumbersome to have a '\n'.join(list) and a split() and strip()
    statement on the other side.
    This proposes to instead have a pretty-print function inside dut-control
    for CLI usage, but allows list, dictionary, etc passing through xmlrpc.
    
    BUG= chromium:834637 
    TEST=manual testing, looks decent with the controls I tried out
    
    Change-Id: Ic896f96f1843a25b9ec23ffdc523115375d5d315
    Reviewed-on: https://chromium-review.googlesource.com/1018501
    Commit-Ready: Ruben Rodriguez Buchillon <coconutruben@chromium.org>
    Tested-by: Ruben Rodriguez Buchillon <coconutruben@chromium.org>
    Reviewed-by: Puthikorn Voravootivat <puthik@chromium.org>
    Reviewed-by: Todd Broch <tbroch@chromium.org>

:040000 040000 fad5220590ffd9e36d9ec877a9bfad2f2faf46fc 1769f7e5bf56d502bc667ac0da548aef6aec532a M	servo

Owner: coconutruben@chromium.org
Status: Assigned (was: Untriaged)
Reverting that patch works, as does applying a hack like this:

diff --git a/servo/system_config.py b/servo/system_config.py
index 2c9cbd1f042a..b10ff4b751aa 100644
--- a/servo/system_config.py
+++ b/servo/system_config.py
@@ -441,6 +441,8 @@ class SystemConfig(object):
       SystemConfigError: errors using formatting param
     """
     if 'map' not in params and 'fmt' not in params:
+      if value is None:
+          return "None"
       return value
     reformat_value = str(value)
     if 'map' in params:

I'm not sure what the right way to unwind this is.
on tot with soraka and dru I get none instead of None as my output.
Is this using dut-control, or using some of the autotest xmlrpc proxies?
I noticed that some of our xmlrpc proxies have allow_none=True not set (in autotest for instance) so that might be causing issues.
I'll add a CL to mitigate the None for now, but I'd want to make sure we can get rid of that special case
dut-control, as the $subject says
What servo device are you using? micro, ccd, v2?

CCD (servo v4). Sorry, forgot to note that
Cc: aaboagye@chromium.org
And BTW, I don't see how Cr50 could every provide a non-None parity:

https://chromium.googlesource.com/chromiumos/third_party/hdctools/+/master/servo/drv/cr50.py#67

    if not hasattr(self._interface, '_ec_uart_bitbang_props'):
      self._interface._ec_uart_bitbang_props = {
          'enabled': False,
          'parity': None,
          'baudrate': None
      }
Blockedon: 841097
crrev.com/c/1051148 for the parity issue
crrev.com/c/1051147 to disallow none return types
Project Member

Comment 10 by bugdroid1@chromium.org, May 11 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/hdctools/+/b5fe0f1391d8d19f9788e38bd96c337677ef666d

commit b5fe0f1391d8d19f9788e38bd96c337677ef666d
Author: Ruben Rodriguez Buchillon <coconutruben@chromium.org>
Date: Fri May 11 06:18:03 2018

sysconfig: don't allow raw None return

Even though we have allow_none=True specified, there's still
issues with None values across xmlrpc. Disallow None return
by casting it to a string (as done previously).

BUG=chromium:840961
BUG=chromium:841097
TEST=manual test
cr50 ccd servod
dut-control -p $PORT ec_uart_parity
ec_uart_parity:None

Change-Id: I505cdaf47dd04db60970743588aec654efce2c3e
Reviewed-on: https://chromium-review.googlesource.com/1051147
Commit-Ready: Ruben Rodriguez Buchillon <coconutruben@chromium.org>
Tested-by: Ruben Rodriguez Buchillon <coconutruben@chromium.org>
Reviewed-by: Todd Broch <tbroch@chromium.org>

[modify] https://crrev.com/b5fe0f1391d8d19f9788e38bd96c337677ef666d/servo/servo_server.py
[modify] https://crrev.com/b5fe0f1391d8d19f9788e38bd96c337677ef666d/servo/system_config.py

Is this bug done? I still see this outstanding:

https://chromium-review.googlesource.com/c/chromiumos/third_party/hdctools/+/1051148

Sign in to add a comment