Needs more discussion and testing
So far it has been working fine for me. The Marlin fw is one that relies in !Serial… and it correctly waits until I open the COM port in Windows before returning true. I have checked it with the debugger also, seeing how it sits there in the loop waiting.
I have noticed though, and this may be a problem of the Sloeber serial monitor, that I lose a few bytes once the port opens. It’s not related to this change though so I think it should go ahead.
There is a PR from Steve to fix issues with the SerialUSB and from reading some of the PR notes it aimed and correcting the lose of data during transfers perhaps losing those bytes could be solved by that other PR?
Steve, was your PR related to losing data when sending from the sketch to the host? I think either the serial monitor or the Windows driver are not picking data fast enough right after the port is opened, and some is dumped from the buffer.
Which bytes are you losing? Randomly or always the first n-bytes?
Arduino IDEs serial monitor has the bad habit to open in the background before you press the monitor open button. But it should show all the data, even it was previously sent before you pressed the open button.
[stevestrong – Tue Jul 04, 2017 12:25 pm] –
Victor, I am not sure that your byte loss issue over USB is related to this. My USB serial fix was long time ago committed to the main branch.Which bytes are you losing? Randomly or always the first n-bytes?
Arduino IDEs serial monitor has the bad habit to open in the background before you press the monitor open button. But it should show all the data, even it was previously sent before you pressed the open button.
I lose a random number, always the first few, but not always lose any bytes. I dont think is related to this change at all, but this change just shows it because the port start sending as soon as the monitor opens the communication. I have only tested with the eclipse monitor, so I think I should test with another monitor too.
I think this change is all good. I have been using it, and I have monitored it with the debugger with a breakpoint and always returns the correct value if and only if the port has been opened in the computer.
So I do not relate those few bytes to this change, was just commenting that with this that issue has become apparent since the board sends as soon as ready for it.
Regarding your changes I didn’t know they had been commited since I was seeing one PR still open for the SPI and USB, and I knew the SPI ones had been separated. I’ll try to write a readBytes function for our driver.
[victor_pv – Tue Jul 04, 2017 3:06 pm] –
I’ll try to write a readBytes function for our driver.
Do you mean a readBytes function for the USB driver? There is already one: https://github.com/rogerclarkmelbourne/ … l.cpp#L162
I think your USB serial byte loss issue should be discussed in other place if it still persists.
For now, I think we all agree that this PR is safe.
I will merge it
Thanks…..
[stevestrong – Tue Jul 04, 2017 5:02 pm] –
The fix that time was manually committed by Roger here: https://github.com/rogerclarkmelbourne/ … 2905fd7ecf[victor_pv – Tue Jul 04, 2017 3:06 pm] –
I’ll try to write a readBytes function for our driver.Do you mean a readBytes function for the USB driver? There is already one: https://github.com/rogerclarkmelbourne/ … l.cpp#L162
I think your USB serial byte loss issue should be discussed in other place if it still persists.
For now, I think we all agree that this PR is safe.
There is a new function called readBytes in teensy and arduino.
I’ll open a separate thread for that one.
It looks like I need to manually merge if Serial because it removes isConnected, which some people may already be using in their sketches
I think I need to keep isConnected by perhaps mark it as deprecated
[RogerClark – Wed Jul 05, 2017 4:45 am] –
Actually,
It looks like I need to manually merge if Serial because it removes isConnected, which some people may already be using in their sketchesI think I need to keep isConnected by perhaps mark it as deprecated
Roger this PR keeps isConnected:
https://github.com/rogerclarkmelbourne/ … 0159d8cR83
It also marks it as deprecated with a nice message advising what to use in its place, so I think you can merge the PR like it is.
I didnt noticed the isConnected had been effectively moved to the header, I thought it has just been deleted from the .c
Its merged now.
Thanks
