patternjavaMinor
Open a comm port and send an ASCII string
Viewed 0 times
opencommasciiportandsendstring
Problem
This is my first ever project and first GUI. I have finished it (with help from Stack Overflow) and it works but is very slow. Can anyone suggest improvements for speed, best practices or usability.?
It currently takes 16 seconds from pressing run in Eclipse to something appearing on my screen. You will need RXTX lib to run it.
This is my GUI:
```
import gnu.io.CommPort;
import gnu.io.CommPortIdentifier;
import gnu.io.NoSuchPortException;
import gnu.io.SerialPort;
import java.util.HashMap;
import java.awt.EventQueue;
import java.awt.event.ItemEvent;
import java.awt.event.ItemListener;
import java.awt.event.MouseAdapter;
import java.io.IOException;
import java.io.OutputStream;
import java.io.PrintStream;
import java.util.ArrayList;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.List;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.border.EmptyBorder;
import javax.swing.JButton;
import java.awt.event.ActionListener;
import java.awt.event.ActionEvent;
import java.awt.GridBagLayout;
import java.awt.GridBagConstraints;
import java.awt.Insets;
import javax.swing.JTextArea;
import javax.swing.JLabel;
import javax.swing.JComboBox;
import javax.swing.SwingUtilities;
import java.awt.Window.Type;
import java.awt.Frame;
import java.awt.Rectangle;
import java.io.IOException;
import javax.swing.JTextField;
import javax.swing.JScrollPane;
public class EBIAlarmUI extends JFrame {
private static final long serialVersionUID = 16022014;
public TwoWaySerialComm twoWaySerCom;
/**
* Start the GUI
*/
public static void main(String[] args) {
EventQueue.invokeLater(new Runnable() {
public void run() {
try {
EBIAlarmUI frame = new EBIAlarmUI();
frame.setVisible(true);
} catch (Exception e) {
e.printStackTrace();
}
}
});
}
/**
* Create the frame.
*/
public EBIAlarmUI() {
initComponents();
twoWaySerCom = new Tw
It currently takes 16 seconds from pressing run in Eclipse to something appearing on my screen. You will need RXTX lib to run it.
This is my GUI:
```
import gnu.io.CommPort;
import gnu.io.CommPortIdentifier;
import gnu.io.NoSuchPortException;
import gnu.io.SerialPort;
import java.util.HashMap;
import java.awt.EventQueue;
import java.awt.event.ItemEvent;
import java.awt.event.ItemListener;
import java.awt.event.MouseAdapter;
import java.io.IOException;
import java.io.OutputStream;
import java.io.PrintStream;
import java.util.ArrayList;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.List;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.border.EmptyBorder;
import javax.swing.JButton;
import java.awt.event.ActionListener;
import java.awt.event.ActionEvent;
import java.awt.GridBagLayout;
import java.awt.GridBagConstraints;
import java.awt.Insets;
import javax.swing.JTextArea;
import javax.swing.JLabel;
import javax.swing.JComboBox;
import javax.swing.SwingUtilities;
import java.awt.Window.Type;
import java.awt.Frame;
import java.awt.Rectangle;
import java.io.IOException;
import javax.swing.JTextField;
import javax.swing.JScrollPane;
public class EBIAlarmUI extends JFrame {
private static final long serialVersionUID = 16022014;
public TwoWaySerialComm twoWaySerCom;
/**
* Start the GUI
*/
public static void main(String[] args) {
EventQueue.invokeLater(new Runnable() {
public void run() {
try {
EBIAlarmUI frame = new EBIAlarmUI();
frame.setVisible(true);
} catch (Exception e) {
e.printStackTrace();
}
}
});
}
/**
* Create the frame.
*/
public EBIAlarmUI() {
initComponents();
twoWaySerCom = new Tw
Solution
Finding Performance Problems
It currently takes 16 seconds from pressing run in Eclipse to something appearing on my screen.
The first step to fixing this is to find out what is causing the delay. My first assumption would be that it is the hardware on the other end of the connection, but who knows. To figure this out, you can do two things:
Before you don't know what the problem is, it will be hard to suggest improvements.
* the communication itself seems to be happening in a different thread, but not the initial connection. As you say you have performance problems on startup, this might be an issue.
Structure
It's really good that you put the connection stuff in it's own
Your fields should also be private and accessed with getters.
The biggest problem with your code structure is your GUI class though: It's way too big, and it has a method with 360 lines, which is just way too much. Nobody is going to read or understand that later on, and it makes your code very hard to extend, debug, profile, reuse, etc.
This gigantic method also contains a lot of duplication, for example the
If you then extract some of the GUI init code that logically belongs to each other to well-named functions, this would already be a great first step.
Misc
It currently takes 16 seconds from pressing run in Eclipse to something appearing on my screen.
The first step to fixing this is to find out what is causing the delay. My first assumption would be that it is the hardware on the other end of the connection, but who knows. To figure this out, you can do two things:
- Mock the
TwoWaySerialCommobject, so that there is no real communication with any external device. If your performance problems go away, you know it's not the GUI, but the external device, or the way you communicate with it*.
- Profile your code and see which methods take the longest.
Before you don't know what the problem is, it will be hard to suggest improvements.
* the communication itself seems to be happening in a different thread, but not the initial connection. As you say you have performance problems on startup, this might be an issue.
Structure
It's really good that you put the connection stuff in it's own
TwoWaySerialComm class. But right now, this class is hard to use, as you are printing in it, and swallowing exceptions. As you have a GUI, it would be nice to report errors in the GUI, not on the command line. So your TwoWaySerialComm class should throw exceptions instead of printing the errors.Your fields should also be private and accessed with getters.
The biggest problem with your code structure is your GUI class though: It's way too big, and it has a method with 360 lines, which is just way too much. Nobody is going to read or understand that later on, and it makes your code very hard to extend, debug, profile, reuse, etc.
This gigantic method also contains a lot of duplication, for example the
alarmXButtonActionPerformed ActionListener are pretty much the same. Create an actual class for them, and pass the button as argument. This would already help quite a bit.If you then extract some of the GUI init code that logically belongs to each other to well-named functions, this would already be a great first step.
Misc
- your naming could be better in parts. Eg it took me a bit to figure out what
gbc_lblParityare. Don't shorten names, and use java naming conventions (camelCase).parityLabelandparityGridBagConstraints(orparityConstraints) aren't all that long, but a lot clearer
Context
StackExchange Code Review Q#85365, answer score: 5
Revisions (0)
No revisions yet.