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

Issue 834637 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 19
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 760267



Sign in to add a comment

servod: support all xmlrpc supported return types

Project Member Reported by coconutruben@chromium.org, Apr 19 2018

Issue description

Right now servod explicitly casts all return types to str before returning them.
https://chromium.googlesource.com/chromiumos/third_party/hdctools/+/master/servo/system_config.py#443

However, as we build more code that uses xmlrpc to directly talk to servod, I'd like to take advantage of the fact that we can pass lists and dictionaries around as well. This is mainly to simplify code both on the returning end of drivers ('\n'.join(list) stuff) and having to split() and strip() the result on the receiving end.

Are there any concerns here that this might break things, or is there a reason why we prefer the current string approach?
 
If we agree then there's a cl here: http://crrev.com/c/1018501
that we can discuss the details of implementation in.
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 28 2018

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

commit 2b8661fb99c9a266429cae324c744db524d4211f
Author: Ruben Rodriguez Buchillon <coconutruben@chromium.org>
Date: Sat Apr 28 01:05:19 2018

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>

[modify] https://crrev.com/2b8661fb99c9a266429cae324c744db524d4211f/servo/dut_control.py
[modify] https://crrev.com/2b8661fb99c9a266429cae324c744db524d4211f/servo/system_config.py
[modify] https://crrev.com/2b8661fb99c9a266429cae324c744db524d4211f/servo/drv/servo_metadata.py

Status: Fixed (was: Started)

Sign in to add a comment