Skip to content

Q: monitor baud rate argument? #1441

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.

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

Closed
4ntoine opened this issue Sep 10, 2021 · 8 comments
Closed

Q: monitor baud rate argument? #1441

4ntoine opened this issue Sep 10, 2021 · 8 comments
Assignees
Labels
conclusion: resolved Issue was resolved topic: documentation Related to documentation for the project type: enhancement Proposed improvement

Comments

@4ntoine
Copy link
Contributor

4ntoine commented Sep 10, 2021

Bug Report

Current behavior

I'm able to connect to my board via gRPC StreamingOpen() request (seems to be). However i'm getting the corrupted output from the board.

I believe it happens due to baud rate not set (not even passed).
How can i pass it (seems that i need to use additional_config). Is it documented?

Another question (just to make sure) what target name means? Is it board port address?

Expected behavior

Correct input is received.

Environment

  • CLI version (output of arduino-cli version): "0.19.0"
  • OS and platform: macOS 10.14.6.

Additional context

It would be good to have all the avaliable additional args description (or source code link).

@per1234
Copy link
Contributor

per1234 commented Sep 10, 2021

How can i pass it (seems that i need to use additional_config).

That's correct. The field name is BaudRate:
https://github.com/arduino/arduino-cli/blob/0.19.x/commands/daemon/monitor.go#L56

Is it documented?

It seems that it is not. I think that part of the problem is that the gRPC interface is intended to be applicable to any type of data source, not only for serial. You can see related work in progress here: #1436

There is a demonstration application here:
https://github.com/arduino/arduino-cli/tree/0.19.x/commands/daemon/term_example
but this is only intended to demonstrate the rate limiting capability, and so uses the "null" type of monitor, so not directly useful as a reference for a serial monitor.

There is a serial demonstration application in a gist that was linked from the original PR that added the Monitor service:
#286
but I think it's a little outdated now.

Here is a version of "term_example" with some small changes to make it print serial data:

// Copyright 2021 ARDUINO SA (http://www.arduino.cc/)
//
// This software is released under the GNU General Public License version 3,
// which covers the main part of arduino-cli.
// The terms of this license can be found at:
// https://www.gnu.org/licenses/gpl-3.0.en.html
//
// You can be released from the requirements of the above licenses by purchasing
// a commercial license. Buying such a license is mandatory if you want to
// modify or otherwise use the software for commercial activities involving the
// Arduino software without disclosing the source code of your own applications.
// To purchase a commercial license, send an email to [email protected].

package main

import (
	"context"
	"fmt"
	"log"
	"time"

	monitor "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/monitor/v1"
	"google.golang.org/grpc"
	"google.golang.org/protobuf/types/known/structpb"
)

var (
	dataDir string
)

func main() {
	conn, err := grpc.Dial("localhost:50051", grpc.WithInsecure(), grpc.WithBlock())
	if err != nil {
		log.Fatal("error connecting to arduino-cli rpc server, you can start it by running `arduino-cli daemon`")
	}
	defer conn.Close()

	// Open a monitor instance
	mon := monitor.NewMonitorServiceClient(conn)
	stream, err := mon.StreamingOpen(context.Background())
	if err != nil {
		log.Fatal("Error opening stream:", err)
	}

	additionalConf, err := structpb.NewStruct(
		map[string]interface{}{"BaudRate": float64(115200)},
	)
	if err != nil {
		log.Fatal("Error creating config:", err)
	}

	if err := stream.Send(&monitor.StreamingOpenRequest{
		Content: &monitor.StreamingOpenRequest_Config{
			Config: &monitor.MonitorConfig{
				Target:              "COM7",
				Type:                monitor.MonitorConfig_TARGET_TYPE_SERIAL,
				AdditionalConfig:    additionalConf,
				RecvRateLimitBuffer: 1024,
			},
		},
	}); err != nil {
		log.Fatal("Error opening stream:", err)
	}

	if err := stream.Send(&monitor.StreamingOpenRequest{
		Content: &monitor.StreamingOpenRequest_RecvAcknowledge{
			RecvAcknowledge: 5,
		},
	}); err != nil {
		log.Fatal("Error replenishing recv window:", err)
	}

	for {
		r, err := stream.Recv()
		if err != nil {
			log.Fatal("Error receiving from server:", err)
		}
		if l := len(r.Data); l > 0 {
			fmt.Print(string(r.Data))
		}
		if r.Dropped > 0 {
			fmt.Printf("DROPPED %d bytes!!!\n", r.Dropped)
		}
		if err := stream.Send(&monitor.StreamingOpenRequest{
			Content: &monitor.StreamingOpenRequest_RecvAcknowledge{
				RecvAcknowledge: 1,
			},
		}); err != nil {
			log.Fatal("Error replenishing recv window:", err)
		}
		time.Sleep(5 * time.Millisecond)
	}
}

(configured for 115200 baud and COM7)

what target name means? Is it board port address?

Yes.

@per1234 per1234 added the topic: documentation Related to documentation for the project label Sep 10, 2021
@4ntoine
Copy link
Contributor Author

4ntoine commented Sep 10, 2021

@per1234 thank you so much

@cmaglie
Copy link
Member

cmaglie commented Sep 10, 2021

@4ntoine since you're an early adopter of the gRPC interface (AFAIK you're the first one actually using it!) I think it is right to inform you that we are in the middle of reworking the "monitor" gRPC interface -> #1436

We already merged a big breaking change to add Pluggable Discoveries, affecting mostly the "board upload" and "board list" portions of the API. Now we are going to complete it with Pluggable Monitors: it will be another breaking change.

I know that breaking backward compatibility is a PITA. Unfortunately making a good API from the beginning is very hard, and we opted to make all the breaking changes we need now until we reach a point when the API is stable enough and at that point release a version 1.0.0 from which we guarantee backward compatibility (the CLI versioning is in alpha 0.x.x for this reason).

@4ntoine
Copy link
Contributor Author

4ntoine commented Sep 10, 2021

BTW i thought Arduino IDE Pro uses gRPC, doesn't it?

@4ntoine
Copy link
Contributor Author

4ntoine commented Sep 10, 2021

Another question: what's the magic about sending "RecvAcknowledge"? I've uploaded Serial echo script and it loops (sends what is received all the time (RX/TX flashing all the time)) if sending "RecvAcknowledge: 1" as a receive confirmation:

void setup() {
  // put your setup code here, to run once:
  Serial.begin(57600);
  Serial.print("hello from serial echo");
  
  pinMode(LED_BUILTIN, OUTPUT);
  digitalWrite(LED_BUILTIN, LOW);
}

// loop 2
void loop() {
  if (Serial.available()) {
    digitalWrite(LED_BUILTIN, HIGH);
    Serial.write(Serial.read());
  }
}

@cmaglie
Copy link
Member

cmaglie commented Sep 10, 2021

BTW i thought Arduino IDE Pro uses gRPC, doesn't it?

sure, I mean the first one outside Arduino organization :-)

Another question: what's the magic about sending "RecvAcknowledge"? I've uploaded Serial echo script and it loops (sends what is received all the time (RX/TX flashing all the time)) if sending "RecvAcknowledge: 1" as a receive confirmation:

It's a rate-limiter, we implemented to solve some performance issues we had on the IDE2 serial monitor.

The first RecvAcknowledge: 5 tells the gRPC server that it must send max 5 messages if the client does not confirm them.
When the client process a message it may confirm it by sending a RecvAcknowledge: 1 that tells the gRPC server: "ok you can now send me 1 more message".
RecvRateLimitBuffer: 1024 tells the gRPC server that the maximum size of a data message must be 1024, so in the example above the maximum amount of data that is buffered in the gRPC serves is 5120 bytes (5 msg * 1024 bytes). If this data is not consumed by the client quickly enough the CLI will start dropping.

If you're not interested in rate limiting, you can just remove RecvRateLimitBuffer and RecvAcknowledge, it should work just fine (only without rate-limits).

@4ntoine
Copy link
Contributor Author

4ntoine commented Sep 10, 2021

@cmaglie good to know, thanks

@4ntoine 4ntoine closed this as completed Sep 10, 2021
@cmaglie cmaglie reopened this Sep 10, 2021
@per1234 per1234 added the type: enhancement Proposed improvement label Mar 31, 2022
@umbynos
Copy link
Contributor

umbynos commented Dec 14, 2022

Hi @4ntoine know a pluggable monitor has been implemented and you should be able to modify the baudrate. Closing this issue as resolved, If you have any more questions feel free to comment further

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conclusion: resolved Issue was resolved topic: documentation Related to documentation for the project type: enhancement Proposed improvement
Projects
None yet
Development

No branches or pull requests

4 participants