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

Add port initialization and validation for SerialCommunicator #22

Open
wants to merge 1 commit into
base: GUIelectron
Choose a base branch
from

Conversation

sabicalija
Copy link
Contributor

  1. test all serial devices for AT COM support
  2. proceed with first valid port

I've implemented the search strategy as discussed today. The easiest solution I could come up with is to provide a init() function for SerialCommunicator.

...
if (userAgent.indexOf(' electron/') > -1) {
    var promise = new Promise(function(resolve) {
        _communicator = new SerialCommunicator();
        _communicator.init().then(function () {
            resolve();
        });
    });
} else {
...

The init() function returns a Promise which is resolved after the correct port is set.

I've changed the communication with the serial device a bit. Right now the program tests if the serial device responds with FABI v2.3 after an AT ID command. This should be changed of course for the correct device (i.e., FlipMouse).


Wouldn't it be better to test for multiple FlipMouse devices and let the user select which one should be used?

1. test all serial devices for AT COM support
1. proceed with first valid port
Copy link
Collaborator

@benjaminaigner benjaminaigner left a comment

Choose a reason for hiding this comment

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

Really nice work, thanks @sabicalija 👍

console.log(
`Found AT COM device at ${comName}`
);
port.close();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we use the parser event for a fully received line?
parser.on('data' ...

Copy link
Contributor Author

@sabicalija sabicalija Dec 6, 2019

Choose a reason for hiding this comment

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

Oh. I didn't know about this possibility.

Sure. I think you're right. It's probably better to safeguard the communication/transmission somehow.

Comment on lines +97 to +104
this.getPorts = function() {
if(_port) {
var list = new Array();
_port.list().forEach( function(port) {
list.push(port['path'] + port['manufacturer']);
});
return list;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can remove this block?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep

@sabicalija
Copy link
Contributor Author

We should eventually also think about how the app should behave when no device is connected or connected after the electron app has been started.

Currently, you'll only get some logs/errors in the console. However, you cannot restart the search after connecting a device.

image

@benjaminaigner
Copy link
Collaborator

also one case that can happen, a port which is not closed correctly:
Screenshot from 2019-12-06 15-26-15

-> I think the FLipMouse is still sending the live values (is started by the GUI with AT SR; values are used for live display of the sensor data), because the AT ER command to stop the raw value reporting is not sent.

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.

2 participants