mhay
March 14, 2020, 8:34am
1
Hi all
I'm part way through a polargraph project that I'm going to run off my laptop running Linux mint.
I'm hunting down a strange issue where the serial monitor is flashing or greying out about once a second even with no MC connected to a usb port.....
I've change the dialout permissions thing:
Its fine connecting to the IDE on windows so the server sketch and cable is fine?
As far as I can remember the IDE was working fine before?
video of the flashing :flashing serial monitor
any suggestions, I've no idea....
pert
March 14, 2020, 11:29am
2
I see it flashes at about 1 Hz, which sounds similar to:
opened 10:11AM - 19 Feb 20 UTC
closed 04:37PM - 23 Mar 20 UTC
Type: Bug
Component: IDE Serial monitor
I've noticed an issue where the serial port gets closed unexpectedly by the seri… al monitor. To reproduce:
- Attach two USB serial ports (e.g. two Arduinos)
- Open up the IDE serial monitor for one of them
- Unplug the other
- The serial monitor will suspend (become disabled, close the serial port) and resume (reopen serial port, reset Arduino if applicable).
This problem does not seem to occur always, but often, because there's a race condition involved.
Under the hood, what happens is:
- Every second, `SerialDiscovery` [refreshes the list of serial ports](https://github.com/arduino/Arduino/blob/15133a072044a822990ab77944434b3a1e134cf6/arduino-core/src/cc/arduino/packages/discoverers/serial/SerialDiscovery.java#L92-L100)
- Also [every second](https://github.com/arduino/Arduino/blob/15133a072044a822990ab77944434b3a1e134cf6/app/src/processing/app/AbstractMonitor.java#L94-L95), `AbstractMonitor` [checks to see if the port is still there (or if it went away, to see if it returned)](https://github.com/arduino/Arduino/blob/15133a072044a822990ab77944434b3a1e134cf6/app/src/processing/app/AbstractMonitor.java#L77-L92).
- These timers fire pretty much at the same time, so they run concurrently. This means that `AbstractMonitor` requests a list halfway through the refresh done by `SerialDiscovery`. This should be fixed by adding some synchronized statements.
- The window for this race condition is enlarged by another bug. During refresh, `SerialDiscovery` incorrectly matches full port strings returned by liblistserials (including vidpid, e.g. `/dev/ttyACM0_16D0_0F44`) against `BoardPort.toString()` (which returns just the port path without the vidpid). This means that whenever the port list changes, it treats all ports as being disconnected and reconnected as new, unique, ports.
- In particular, this causes *all* existing ports to be [marked as offline early in the refresh process](https://github.com/arduino/Arduino/blob/15133a072044a822990ab77944434b3a1e134cf6/arduino-core/src/cc/arduino/packages/discoverers/serial/SerialDiscovery.java#L127-L138). If `AbstractMonitor` then requests a list of (online) serial ports, this will return the empty list, causing the monitor to close.
The minimal fix, I suspect, would be:
- Fix the port name comparison by splitting the string returned by liblistserial early, and then use just the port name for comparisons.
- Make all access of [`serialBoardPorts`](https://github.com/arduino/Arduino/blob/15133a072044a822990ab77944434b3a1e134cf6/arduino-core/src/cc/arduino/packages/discoverers/serial/SerialDiscovery.java#L44) synchronized (on second thought: Maybe better to actually *replace* the `serialBoardPorts` array completely rather than [clearing and refilling it](https://github.com/arduino/Arduino/blob/15133a072044a822990ab77944434b3a1e134cf6/arduino-core/src/cc/arduino/packages/discoverers/serial/SerialDiscovery.java#L73-L74) when it changes, since that is probably also atomic, and also makes sure that any callers that have a previous version of the list will not see their list change all of a sudden (on closer inspection, it seems that a copy is [already returned now](https://github.com/arduino/Arduino/blob/15133a072044a822990ab77944434b3a1e134cf6/arduino-core/src/cc/arduino/packages/discoverers/serial/SerialDiscovery.java#L61), so this does not really matter).
However, looking around this code, it does seem like it's a bit of mess. I would think some refactoring might be helpful, except that I suspect that (serial) port discovery will soon be delegated to `arduino-cli`? @cmaglie, I suspect you can comment on that?
Regardless, some things I think could be improved are:
- Instead of [listing ports and looking in the list for the serial monitor port](https://github.com/arduino/Arduino/blob/15133a072044a822990ab77944434b3a1e134cf6/app/src/processing/app/AbstractMonitor.java#L81-L89), one could simply look at the `boardPort.isOnline()` which (if the above is fixed) should switch to false and back to true.
- Add an event handler for `isOnline` changes instead of having a timer in `AbstractMonitor`, to make sure a quick offline/online transition is not missed.
- Currently, serial port listing [returns a port name, vid and pid](https://github.com/arduino/Arduino/blob/15133a072044a822990ab77944434b3a1e134cf6/arduino-core/src/cc/arduino/packages/discoverers/serial/SerialDiscovery.java#L142), but a few lines further down, `resolveDeviceByVendorIdProductId()` is called which [again looks up the vidpid for the port name](https://github.com/arduino/Arduino/blob/01d3d02a0fca6c80d76163274c077687c05f1d76/arduino-core/src/processing/app/Platform.java#L180). The latter could probably be skipped and just use the vidpid that are already known. *Update: On closer inspection, it seems that the latter actually also returns info on the serial number and product name, while the serial port list only has vidpi.*
- I think there is [some substring matching](https://github.com/arduino/Arduino/blob/01d3d02a0fca6c80d76163274c077687c05f1d76/arduino-core/src/processing/app/Platform.java#L190-L194) that could end up with false positives iSerial contains something that looks like a PID, or something like that.
though in that case the "flash" is caused by disconnecting a different port, so not exactly the same.
The first thing I would recommend trying is updating to Arduino IDE 1.8.12. It may be that the bug you're encountering has been fixed since the release of the Arduino IDE 1.8.10 you're using. If it still occurs with 1.8.12, you might try the test build provided by ArduinoBot in the comment thread of this pull request:
arduino:master ← facchinm:serial_event_detaches_all_ports
opened 05:56PM - 10 Mar 20 UTC
Fixes #9785 and probably many others
This commit strongly simplifies the seri… al list code
Pluggable discovery introduced a bug since `BoardPort.toString()` started reporting only the name of the port, not the complete `name_vid_pid` needed to match `liblistserial` output.
Adding `.toCompleteString()` almost solves the bogus disconnection part alone, but `resolveDeviceByVendorIdProductId()` uses "0x" prefixes VID/PID, breaking it again.
In addition, all the logic used to match a board with its bootloader (to obtain a serial number on 32u4 boards) has been completely removed since it is currently useless (and unused).
### All Submissions:
* [x] Have you followed the guidelines in our Contributing document?
* [x] Have you checked to ensure there aren't other open [Pull Requests](https://github.com/arduino/Arduino/pulls?q=) for the same update/change?
### New Feature Submissions:
1. [x] Does your submission pass tests?
2. [x] Have you lint your code locally prior to submission?
### Changes to Core Features:
* [x] Have you added an explanation of what your changes do and why you'd like us to include them?
* [ ] Have you written new tests for your core changes, as applicable?
* [x] Have you successfully ran tests with your changes
