-
Notifications
You must be signed in to change notification settings - Fork 0
Implement Bluetooth Sensor #317
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
Conversation
cal-pratt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only concern is closing the inputstream of the serial port object during calls to onNext. Apart from that and a few nitpicks LGTM
Ship it.
| public class BluetoothViewController implements ViewController { | ||
| private final BluetoothView view; | ||
|
|
||
| private Observable<BluetoothValue> bluetoothValues; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be final.
| final String comPortName, | ||
| final String comPort, | ||
| final int connectionTimeout, | ||
| final int baudRate) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick:
I've been opting for single tabs in all indent cases and to place the ( on a new line to distinguish where the parameters end and the method begins.
public BluetoothReader(
final String comPortName,
final String comPort,
final int connectionTimeout,
final int baudRate
) {
this.comPortName = comPortName;
this.comPort = comPort;
this.connectionTimeout = connectionTimeout;
this.baudRate = baudRate;
}| try { | ||
| try (final InputStream inputStream = serialPort.getInputStream(); | ||
| final Scanner scanner = new Scanner(inputStream) | ||
| ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will calls to close on the inputstream cause the serial port to close?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe don't use the inputstream as a try-with resource and wait for the proper cleanup in onUnsubscribe which will close all connections from/to the serial port.
| protected SerialPort next(final SerialPort serialPort, final Observer<? super BluetoothValue> observer) { | ||
| try { | ||
| final InputStream inputStream = serialPort.getInputStream(); | ||
| try (final Scanner scanner = new Scanner(inputStream)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this will also kill the input. Calling close on a wrapped stream usually closes the initial stream.
|
The test implementation i had called scanner.close and it worked fine. We'll confirm it when it's tested on hardware
… On Jun 11, 2017, at 8:59 PM, Cal Pratt ***@***.***> wrote:
@cal-pratt commented on this pull request.
In src/main/java/com/easternedgerobotics/rov/io/BluetoothReader.java:
> + throw new RuntimeException(e);
+ }
+ }
+
+ /**
+ * Read the next line of input from the serial port if it exists.
+ *
+ * @param serialPort the bluetooth serialPort.
+ * @param observer the listener to this receiver.
+ * @return the serial port
+ */
+ @OverRide
+ protected SerialPort next(final SerialPort serialPort, final Observer<? super BluetoothValue> observer) {
+ try {
+ final InputStream inputStream = serialPort.getInputStream();
+ try (final Scanner scanner = new Scanner(inputStream)) {
Not sure if this will also kill the input. Calling close on a wrapped stream usually closes the parent stream.
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
#242
On the ROV side, the BluetoothReader listens for messages on the bluetooth serial port and broadcasts BluetoothValues
On Topsides, the BluetoothView is a console view that displays each emitted BluetoothValue