Skip to content

[WIP] feat: Add a copy button to serial monitor #2718

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

502E532E
Copy link

Motivation

When collecting data with the Arduino, a straight-forward way to retrieve it is to write it to the serial monitor (often in csv format or similar) and store it somewhere.

As addressed in various issues (#1081, #2093, #812), retrieving this data from the serial monitor is hard to impossible currently, as it does not really support copying, nor has the functionality to write the data to a file.

Change description

This pull request introduces a button that copies the content of the serial monitor to the clipboard. It uses the raw data stored in the frontend (an array of strings/lines) and joins it to a single string that is copied. This avoids the issue of formatted text completely (#2093) and makes it easy to copy the full content (#1081, #812).

Other information

In the long run (and in order to fully resolve the issues above), a proper copy mechanism of the current selection should be implemented. But this seems to be hard, some related issue (#105) is almost five years old and still open, although there was some implementation effort.

This pull requests aim is to provide some small quality of life feature. It might be replaced by some farther reaching change in the future, or might even stay as it is useful as a quick action nonetheless.

Feedback needed

I created this pull request because I want to use the feature myself. I do not really understand the code structure and style conventions of this repository (Is there some style guide document? I could not find it), so I need feedback on these aspects.

The code itself is working as I would want to use it, allowing to copy the content of the serial monitor to the clipboard. There is a test for the method that joins the lines, I am not sure if the GUI components require tests.

  • Is the functionality at the right location? The copying happens in serial-monitor-send-output, because it seems this is the only place where the lines are actually stored. It feels a bit wrong to do it there, though
  • Are more tests required?
  • Is there some documentation that needs updating?
  • Is the localization key reasonable? Does it need further action?

Reviewer checklist

  • PR addresses a single concern.
  • The PR has no duplicates (please search among the Pull Requests before creating one)
  • PR title and description are properly filled.
  • Docs have been added / updated (for bug fixes / features)

If the arduino collects some data that you want to store on your
computer, a rather simple way is to write it to the serial monitor and
copy it to the clipboard. This commit introduces a button that copies
the whole content of the serial monitor to the clipboard to make this
rather simple. It is a new component added to the menu, and does not
change the behaviour of other compontents.
Adds a test for merging one or more lines to a single string. It is
supposed to just concatenate the content of the lines, without doing
anything else. This method is used when copying the serial monitor content to
the clipboard.
@per1234 per1234 added type: enhancement Proposed improvement topic: code Related to content of the project itself topic: serial monitor Related to the Serial Monitor labels Apr 23, 2025
This serves as an addition to the previous commits. It is the result of
running `yarn i18n:generate` on the state after adding the copy output
button to the serial monitor (see 2df3f46). I hope that this will
resolve the current Github action failure.
Copy link
Contributor

@dankeboy36 dankeboy36 left a comment

Choose a reason for hiding this comment

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

I checked the code and verified the behavior with #2093 (comment), and a speed test sketch, it worked for me.

/*
  Simple test of Serial.print() speed.  How many lines/sec can your
  Arduino board print to the serial monitor?
  
  This combination of printing text and numbers is meant to test the
  most common way Serial printing is used with Arduino.
  
  On true serial boards, this test measures how much of the bandwidth
  of a high baud rate can really be used.  Efficiency copying bytes
  into the transmit buffer, and the serial transmit interrupt, are
  the main factors.
  
  On native USB boards, this test measures how well the USB stack can
  utilize the USB bandwidth.  USB is fast but complex protocol.  This
  test tends to measure how well optimized the USB code is.  For
  12 Mbit/sec (full speed) USB, 36848 is the theoretical maximum
  lines/sec.  For 480 Mbit/sec (high speed) USB, 1521371 is the
  theoretical maximum lines/sec speed.  Unlike ordinary serial, your
  operating system and its drivers, the type of USB port, whether a
  USB hub is used, and CPU load on your computer can affect the speed.
*/

uint32_t count, prior_count;
uint32_t prior_msec;
uint32_t count_per_second;

// Uncomment this for boards where "SerialUSB" needed for native port
//#define Serial SerialUSB

void setup() {
  Serial.begin(115200);  // edit for highest baud your board can use
  while (!Serial)
    ;
  count = 10000000;  // starting with 8 digits gives consistent chars/line
  prior_count = count;
  count_per_second = 0;
  prior_msec = millis();
}

void loop() {
  Serial.print("count=");
  Serial.print(count);
  Serial.print(", lines/sec=");
  Serial.print(count_per_second);
  Serial.print(", compilation timestamp=");
  Serial.print(__TIME__);
  Serial.print(", elapsed=");
  Serial.print(millis());
  Serial.println(" ms");
  count = count + 1;
  uint32_t msec = millis();
  if (msec - prior_msec > 1000) {
    // when 1 second as elapsed, update the lines/sec count
    prior_msec = prior_msec + 1000;
    count_per_second = count - prior_count;
    prior_count = count;
  }
}
arduino-ide.2093.mov

Great work! People will love this QoL improvement. I left a few remarks in the code. I propose dropping the [WIP] prefix from the PR title.

Comment on lines +71 to +73
export function linesToMergedStr(lines: Line[]) : string {
return lines.map((line: Line) => {return line.message}).join("");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
export function linesToMergedStr(lines: Line[]) : string {
return lines.map((line: Line) => {return line.message}).join("");
}
export function linesToMergedStr(lines: Line[]): string {
return lines.map((line: Line) => line.message).join('');
}

A simple stringifyLines or joinLines would be a more appropriate function name, maybe

Comment on lines +55 to +62
export const COPY_OUTPUT = Command.toLocalizedCommand(
{
id: 'serial-monitor-copy-output',
label: 'Copy Output',
iconClass: codicon('copy'),
},
'arduino/serial/copyOutput'
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
export const COPY_OUTPUT = Command.toLocalizedCommand(
{
id: 'serial-monitor-copy-output',
label: 'Copy Output',
iconClass: codicon('copy'),
},
'arduino/serial/copyOutput'
);
export const COPY_OUTPUT = {
id: 'serial-monitor-copy-output',
};

Attaching a label and iconClass to this command at this level is unnecessary. A label and iconClass are required for a command if the IDE wants to show it in the Command Palette, but it's not the case here; it will be shown on the toolbar or the Monitor widget. Registering the command with the id, and doing the label and icon classes for the toolbar item is sufficient. See the corresponding proposed change below 👇

Comment on lines +160 to +167
registry.registerItem({
id: SerialMonitor.Commands.COPY_OUTPUT.id,
command: SerialMonitor.Commands.COPY_OUTPUT.id,
tooltip: nls.localize(
'arduino/serial/copyOutput',
'Copy Output'
),
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
registry.registerItem({
id: SerialMonitor.Commands.COPY_OUTPUT.id,
command: SerialMonitor.Commands.COPY_OUTPUT.id,
tooltip: nls.localize(
'arduino/serial/copyOutput',
'Copy Output'
),
});
registry.registerItem({
id: SerialMonitor.Commands.COPY_OUTPUT.id,
command: SerialMonitor.Commands.COPY_OUTPUT.id,
icon: codicon('copy'),
tooltip: nls.localize('arduino/serial/copyOutput', 'Copy Output'),
});

Comment on lines +109 to +113

copyOutput(): void {
this.copyOutputEmitter.fire();
this.update();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
copyOutput(): void {
this.copyOutputEmitter.fire();
this.update();
}
copyOutput(): void {
this.copyOutputEmitter.fire();
}

It's not needed to update the widget after copying the text; no need to rerender.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself topic: serial monitor Related to the Serial Monitor type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants