Skip to content

RSDK-13909: Update CLI machine shell for windows #6030

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
allisonschiang wants to merge 4 commits into
base: main
Choose a base branch
from
+12 −16
Open

RSDK-13909: Update CLI machine shell for windows #6030

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5d98cfc
cli: replace stty shell-out with golang.org/x/term in machine part shell
allisonschiang May 5, 2026
39564ab
Merge branch 'main' of https://github.com/viamrobotics/rdk into cli-f…
allisonschiang May 19, 2026
024d877
Merge branch 'main' of https://github.com/viamrobotics/rdk into cli-f…
allisonschiang May 20, 2026
421fa68
propogate error
allisonschiang May 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 5 additions & 15 deletions cli/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import (
"go.viam.com/utils"
"go.viam.com/utils/protoutils"
"go.viam.com/utils/rpc"
"golang.org/x/term"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/metadata"
reflectpb "google.golang.org/grpc/reflection/grpc_reflection_v1alpha"
Expand Down Expand Up @@ -5336,24 +5337,13 @@ func (c *viamClient) startRobotPartShell(
})
}

setRaw := func(isRaw bool) error {
// NOTE(benjirewis): Linux systems seem to need both "raw" (no processing) and "-echo"
// (no echoing back inputted characters) in order to allow the input and output loops
// below to completely control the terminal.
args := []string{"raw", "-echo", "-echoctl"}
if !isRaw {
args = []string{"-raw", "echo", "echoctl"}
}

rawMode := exec.Command("stty", args...)
rawMode.Stdin = os.Stdin
return rawMode.Run()
}
if err := setRaw(true); err != nil {
stdinFd := int(os.Stdin.Fd())
Copy link
Copy Markdown
Member Author

@allisonschiang allisonschiang May 20, 2026

Choose a reason for hiding this comment

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

stty doesn't work on windows - this alternative does

oldTermState, err := term.MakeRaw(stdinFd)
if err != nil {
return err
}
defer func() {
utils.UncheckedError(setRaw(false))
utils.UncheckedError(term.Restore(stdinFd, oldTermState))
}()

utils.PanicCapturingGo(func() {
Expand Down
8 changes: 7 additions & 1 deletion services/shell/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package shell
import (
"context"
"errors"
"io"
"sync"

pb "go.viam.com/api/service/shell/v1"
Expand Down Expand Up @@ -131,9 +132,14 @@ func (c *client) Shell(
for {
resp, err := client.Recv()
if err != nil {
errMsg := ""
Copy link
Copy Markdown
Member Author

@allisonschiang allisonschiang May 20, 2026

Choose a reason for hiding this comment

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

error msg would not propogate when trying to connect to windows and it would just silently fail, so I added this here so our "windows isn't supported" error we already have is visible

if !errors.Is(err, io.EOF) {
errMsg = err.Error()
}
select {
case output <- Output{
EOF: true,
Error: errMsg,
EOF: true,
}:
case <-ctx.Done():
}
Expand Down
Loading