Skip to content

Fix SerialPlotter.java not applying line ending preference on new window to dropdown box #11070

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

Merged
merged 1 commit into from
May 10, 2021

Conversation

sceniclife
Copy link
Contributor

@sceniclife sceniclife commented Dec 14, 2020

applyPreferences is called but the class function is misspelled as appyPreferences

Because of the typo, the class function was never called and only the superclass is called.
In practice, the applyPreferences is just reading the preferences file and setting the dropdown boxes to the current running value. Because the class applyPreferences wasn't called, the line ending dropdown box is not displaying the correct line ending that the user had set.

Before the change, changing the line ending preference does change the preference.

lineEndings.addActionListener((ActionEvent event) -> {
PreferencesData.setInteger("serial.line_ending", lineEndings.getSelectedIndex());
noLineEndingAlert.setForeground(pane.getBackground());
});

However, it's possible that No line ending is used even if you had a line ending preference set internally. (send() class function uses the current dropdown selection, and because No line ending is the default value on new window)

private void send(String string) {
String s = string;
if (serial != null) {
switch (lineEndings.getSelectedIndex()) {
case 1:
s += "\n";
break;
case 2:
s += "\r";
break;
case 3:
s += "\r\n";
break;
default:
break;
}
if ("".equals(s) && lineEndings.getSelectedIndex() == 0 && !PreferencesData.has("runtime.line.ending.alert.notified")) {
noLineEndingAlert.setForeground(Color.RED);
PreferencesData.set("runtime.line.ending.alert.notified", "true");
}
serial.write(s);
}
}

@per1234 per1234 added the SerialPlotter Tools > Serial Plotter label Dec 15, 2020
Copy link
Collaborator

@matthijskooijman matthijskooijman left a comment

Choose a reason for hiding this comment

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

Huh? You rename a method, but no calls to this method? It seems it was called under the old name, how does that work? Is it that the superclass defines the method by the proper name and SerialPlotter tried but failed to override this method due to the typo?

What does this mean in practice? Before this fix, the line endings preference was not applied to the plotter? Or was it just that the dropdown did not reflect the actual value, but the right value was used internally?

@sceniclife
Copy link
Contributor Author

sceniclife commented Dec 15, 2020

@matthijskooijman The method is called on line

Because of the typo, the class function was never called and only the superclass is called.
In practice, the applyPreferences is just reading the preferences file and setting the dropdown boxes to the current running value. Because the class applyPreferences wasn't called, the line ending dropdown box is not displaying the correct line ending that the user had set.

Before the change, changing the line ending preference does change the preference.

lineEndings.addActionListener((ActionEvent event) -> {
PreferencesData.setInteger("serial.line_ending", lineEndings.getSelectedIndex());
noLineEndingAlert.setForeground(pane.getBackground());
});

However, it's possible that No line ending is used even if you had a line ending preference set internally. (send() class function uses the current dropdown selection, and because No line ending is the default value on new window)

private void send(String string) {
String s = string;
if (serial != null) {
switch (lineEndings.getSelectedIndex()) {
case 1:
s += "\n";
break;
case 2:
s += "\r";
break;
case 3:
s += "\r\n";
break;
default:
break;
}
if ("".equals(s) && lineEndings.getSelectedIndex() == 0 && !PreferencesData.has("runtime.line.ending.alert.notified")) {
noLineEndingAlert.setForeground(Color.RED);
PreferencesData.set("runtime.line.ending.alert.notified", "true");
}
serial.write(s);
}
}

@sceniclife sceniclife changed the title Typo in SerialPlotter.java Fix SerialPlotter.java not applying line ending preference on new window to dropdown box Dec 15, 2020
@sceniclife
Copy link
Contributor Author

Change can also be seen here: #7461

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@cmaglie cmaglie merged commit 69e91ca into arduino:master May 10, 2021
@cmaglie cmaglie added this to the Release 1.8.14 milestone May 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SerialPlotter Tools > Serial Plotter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants