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

Issue 825692 link

Starred by 2 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

servod: support multi-instance servod

Project Member Reported by coconutruben@chromium.org, Mar 26 2018

Issue description

Right now, we have some messy-ish logic going on to support what is essentially multiple servod instances controlled by one instance.

https://chromium.googlesource.com/chromiumos/third_party/hdctools/+/master/servo/servo_postinit.py

I think a cleaner solution would be to build something along the lines of
sudo multiservod -b soraka -c soraka_rev_3.xml -- --prefix hammer -b hammer

where under the hood we get a
- servod instance for soraka
- one for hammer
- and one that dispatches e.g. all commands that start with 'hammer_' will get that stripped away and will be sent to the hammer servod and otherwise they get sent to the main servod.

It would also be beneficial to add support for this to happen after the fact, meaning being able to bind two existing servod instances. That would also simplify the flow for postinit.

What do you guys think? I'd like to build a prototype for this if we agree this is a "good" path to go down, and then we can take it from there to build out all the features this needs to support.
 

Comment 1 by mqg@chromium.org, Mar 26 2018

Could you elaborate more on why this is necessary and what's the use case? Asking because I never used servod like this.

Comment 2 by puthik@google.com, Mar 26 2018

We already have that mechanism.

1. You can name servo soraka and hammer by put this in ~/.servodrc in chroot
# name serial-number port-number board
soraka, xxxxxx-xxxxx, 9900, soraka
hammer, yyyyyy-yyyyy, 9901, hammer

2. Route command to soraka or hammer via "-n" argument in dut-control


I think this solution is already good enough and "-n" is cleaner than prefix every command with hammer_

Comment 3 by tbroch@chromium.org, Mar 26 2018

@#1, the usage in this case is for detachables where there's an individual servo connected to both the lid (soraka) & base (hammer) which presumably are also connected via their prescribed interface (USB) such that we'd want to avoid doing certain things to one w/o the other knowing about it.


@#2, agreed that mechanism should provide a means to join lid & base accordingly.



Thanks for #2, I hadn't looked at multiservo logic yet. 
We had a quick chat about this today, and the concern that Waihong had (and feel free to correct me if I'm explaining this wrong) is that right now the faft pipeline and lab also relies on this assumption that there is one 'servod' instance per device, one server to talk to. So -n just replaces the need for a -p parameter, but ultimately you still don't have one server to talk to.
https://chromium.googlesource.com/chromiumos/third_party/autotest/+/master/server/hosts/servo_host.py#173
So it made sense to me to honor that assumption by having a sort of muxing-server that handles the forwarding & command exposure. How exactly we do that I'm not sure yet. Maybe there's a much cleaner way than the prefixes as Opal pointed out to do the routing.

So far we identified the following use cases that right now use complicated logic:
- controlling base and lid with one servod instance
- controlling base and lid where both are micro servos attached to a v4
- v4's post_init where we make assumptions on the order of config files being loaded
- flash_ec with the v4_and_micro setup, where we want to be able to flash the lid and base ec

The other alternatives would be to leverage command routing in dut-control either by using servorc file, or other means, and rewriting the autotest pipeline to not use a server proxy, but to issue dut-control commands on the cmdline, or create servo_client instances on the fly.

Comment 5 by gkihumba@google.com, Mar 27 2018

Waihong already added the support for one servod process enumerating both base + lid. It's what I'm using to write hammer tests.
Setup:
Servo v4, with ccd on lid and uservo on base. 
'sudo servod -b soraka' brings up controls with hammer_blah for the uservo. If only base/lid is connected it defaults to the controls without a prefix. 
However, if base ec fw is bricked, then the hammer controls don't enumerate (with both base + lid connected). There's another bug to fix this.

We also added support to flash_ec to recognize when a base is connected and look for 'base_' controls. Patch will be uploaded soon.
https://chromium-review.googlesource.com/#/c/chromiumos/platform/ec/+/977322/

#5: Right, but:

 a. The flash_ec patch looks quite terrible
 b. hammer_servo_type is currently broken (see patch)
 c. servod --subboard parameter (b/76185690) will not be very easy to generalize

For staff FAFT, given the time constraints, we can still go with the current approach, but it'd be nice to think if we can do better in the long run.
Pretty sure everyone here is added to that document, but design-doc for the upgrade is at go/servoarch

Comment 8 by tbroch@chromium.org, Jan 18 (4 days ago)

Labels: Pri-3
De-prioritizing to '3' given 'available' status & removing milestones accordingly.  If you disagree please mark 'untriaged'.

Sign in to add a comment