Skip to content
This repository was archived by the owner on Dec 23, 2021. It is now read-only.

Update test message for serial monitor #141

Merged
merged 2 commits into from
Aug 23, 2019

Conversation

jonathanwangg
Copy link
Contributor

Description:

In this PR, I'm updating the message we're sending to test if the serial port is open. Previously, the message wasn't informative enough to the user.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Limitations:

None

Testing:

Can't really reliably test this since we only see the message if the serial port errors out.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

Copy link
Contributor

@smmatte smmatte left a comment

Choose a reason for hiding this comment

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

Are you able to guide the user based on the error code

Copy link
Contributor

@Christellah Christellah left a comment

Choose a reason for hiding this comment

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

The message explains a little better what's happening, we will make sure to let the user know in our troubleshoot doc how they can address it.

@@ -73,7 +73,7 @@ export class SerialPortControl {
this._currentSerialPort = new SerialPortControl.serialport(this._currentPort, { baudRate: this._currentBaudRate });
this._outputChannel.show();
this._currentSerialPort.on("open", () => {
this._currentSerialPort.write("msft", "Both NL & CR", (err: any) => {
this._currentSerialPort.write(CONSTANTS.MISC.SERIAL_MONITOR_TEST_IF_OPEN, "Both NL & CR", (err: any) => {
if (err && !(err.message.indexOf(CONSTANTS.ERROR.COMPORT_UNKNOWN_ERROR) >= 0)) {
logToOutputChannel(this._outputChannel, CONSTANTS.ERROR.FAILED_TO_OPEN_SERIAL_PORT(this._currentPort));
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add after that line another logToOutputChannel telling them to try re-plugging the device?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'm pretty sure we can add another line that prints to an OutputChannel if an error occurs. Do you think logging to the Pacifica Simulator Output Channel is more appropriate?

@jonathanwangg
Copy link
Contributor Author

Are you able to guide the user based on the error code

In my opinion, it would be hard to guide the user based on what the error code is. Best thing to do would be directing the user to unplug the device, re-plug it and try again. Do you think that would be sufficient given our time frame?

@jonathanwangg jonathanwangg merged commit 678b6ac into dev Aug 23, 2019
@jonathanwangg jonathanwangg deleted the users/t-jowang/update-test-message-serial-monitor branch August 26, 2019 09:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants