Skip to content

Conversation

@gbalke
Copy link
Member

@gbalke gbalke commented Sep 26, 2022

Signed-off-by: Greg Balke gbalke@berkeley.edu

Topic: bdt_uses_dcargs
Reviewers: brent

Signed-off-by: Greg Balke <gbalke@berkeley.edu>

Topic: bdt_uses_dcargs
Reviewers: brent
@gbalke
Copy link
Member Author

gbalke commented Sep 26, 2022

Reviews in this chain:
#69 [Bdt] Switch to dcargs

@gbalke
Copy link
Member Author

gbalke commented Sep 26, 2022

# head base diff date summary
0 83f41825 f552a7fd diff Sep 25 9:13 PM 4 files changed, 94 insertions(+), 61 deletions(-)
1 0b7072fc f552a7fd diff Oct 04 12:44 AM 1 file changed, 5 insertions(+), 4 deletions(-)

@gbalke gbalke requested a review from brentyi September 26, 2022 04:13
"""


actuation: dcargs.conf.Positional[str]
Copy link
Collaborator

Choose a reason for hiding this comment

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

probably a literal could be used here?

from typing import Literal

actuation: Literal["current", "phase"], etc



actuation: dcargs.conf.Positional[str]
values: dcargs.conf.Positional[List[Tuple[float, float, float]]]
Copy link
Collaborator

Choose a reason for hiding this comment

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

in general, variable-length positional arguments are scary because it means you can't have add more positional args afterward


board_ids = make_type(make_list(ast.literal_eval(args.board_ids)), int)
board_ids = make_type(
make_list(ast.literal_eval(args.motor.board_ids)), int
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this still needed? since board_ids is annotated as List[int] now

@gbalke
Copy link
Member Author

gbalke commented Oct 4, 2022

Discussed offline... I think the current behavior where betz drive tools creates a connection per tool execution is a pretty poor design for doing continuous testing. I believe dcargs will work much better if it doesn't have to deal with lists of lists of command args (which is the current hacky think I'm doing for control_motor. I'll leave this PR and hold off until I have a better architecture in place for the server system.

@gbalke gbalke force-pushed the gbalke/revup/master/bdt_uses_dcargs branch from 83f4182 to 0b7072f Compare October 4, 2022 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants