Skip to content
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

Closing terminal halts Esruino app. #76

Closed
ZiCog opened this issue Oct 5, 2013 · 13 comments
Closed

Closing terminal halts Esruino app. #76

ZiCog opened this issue Oct 5, 2013 · 13 comments
Labels

Comments

@ZiCog
Copy link

ZiCog commented Oct 5, 2013

With the following code run on Espruino 1v40 on an STM F4 Discovery the LED flashing stops when the terminal program, minicom, is closed.

On reopening the terminal program after a while a rush of text is dumped and the LED toggling runs very fast until all the events have been cleared and things then get back to their intended rate.

Oddly pulling out the USB cable does not have this effect on the LED flashing rate.

var i = setInterval(function () {
on = !on;
digitalWrite(LED1, on);
digitalWrite(LED4, !on);
}, 1000);

var j = setInterval(function () {
print("All work and no play makes Jack a dull boy.");
}, 1000);

@gfwilliams
Copy link
Member

Hmm - this is just flow control I guess, as Espruino is blocking waiting for the PC to read the data (it shouldn't block for small amounts of data but it will when the output buffer gets full). When there is no USB connection active, Espruino knows and it throws away all data destined for USB.

What should the correct behaviour be? If Espruino randomly threw data away when the PC wasn't reading it, that would be a bug IMO - in a way this seems correct. If you're going to have flow control, you've got to expect this...

I guess there is an issue is Espruino prints too much data at startup, and is then plugged in via USB - but I would have thought that if that happened, i'd have noticed by now?

@ZiCog
Copy link
Author

ZiCog commented Oct 5, 2013

There are two orthogonal issues here then:

  1. The setInterval() for flashing the LEDs as above, or anything else, should not be hung up just because the serial output flow is halted. Same goes for any other I/O. That is a bug as it basically stops my whole program unnecessarily. The JavaScript mantra is "do not block". That would apply to output in general not when when it is done in an interval call back.

  2. Should we have flow control or not?

I would say no because:

a) I have worked with dozens of serial connections on embedded systems and rarely have I seen flow control in use. I did not expect it here.

b) Even non-embedded systems often don't use flow control. The Linux console port over serial does not normally buffer up data for example.

c) When it comes to losing data that is often handled by a higher level protocols, could be XON/XOFF or even some application specific protocol. Besides flow control does not ensure you data has actually been received intact.

d) If data loss when the USB is physically disconnected is acceptable why not when the PC is not listening? Randomly throwing away data on a disconnected line is very common behavior, higher level protocols are used to detect and take care of that if necessary.

@gfwilliams
Copy link
Member

  1. It's not a bug. It's just not how Espruino works. It is single-threaded, and as such, if something blocks, everything blocks, Multithreading is not an option because of the memory limitations. If you don't exceed the output buffer size, everything works fine though.

  2. I'd say flow control is vital for the UI - otherwise there will be lost characters (and bug reports) all over the place.

You're welcome to disable it on your version, but I believe not having it will cause problems for far more people than if it existed (for instance copy/paste).

@ZiCog
Copy link
Author

ZiCog commented Oct 6, 2013

  1. It's not a bug provided it is documented behavior.

Traditionally on small real-time systems such serial output was done by having a UARTs empty state trigger an interrupt handler that read the next byte to send from a cyclic buffer. Very small and simple, no "threading" or scheduler needed. Such a cyclic buffer can be made without any mutexes or such like. Similarly for input. Of course data would eventually be lost if the buffers were allowed to become full which is something the total system design would have to account for.

If this blocking behavior is pervasive in Espruino then perhaps it's not worth worrying about this particular case.

  1. Point taken about the UI behavior. In passing I have to say the editor works much better than I imagined it would.

Perhaps this issue could be moved into the application level by providing a means to find out the free space in the buffer prior to printing anything.

@gfwilliams
Copy link
Member

We do have cyclic buffer for output (shared between all USARTs) so simple prints are async - but when it gets full the code blocks rather than dropping characters, which I think is probably what you'd expect.

Having some kind of feedback as to the amount of buffer left sounds like a great idea though. Potentially it could just be reported as another item in 'memory()'.

Also, thinking about it, maybe Espruino should count itself as disconnected from USB if it is not being polled for new character data (even if USB itself is connected). This might solve the problem - although it'd mean that the Espruino logo screen would most likely not get displayed most times.

@gfwilliams
Copy link
Member

This came up again:

#213

I reckon we should allow this to be turned on/off along with the flow control implementation. See here: #20

@bartmichu
Copy link
Contributor

This is weird. If you run the code below without the terminal connected and press the button repeatedly (around 10 times) then it just stops.

setWatch(function () {
  digitalWrite(LED1, 1);
  console.log("on");
}, BTN1, {repeat: true, edge: "rising", debounce: 70});

setWatch(function () {
  digitalWrite(LED1, 0);
  console.log("off");
}, BTN1, {repeat: true, edge: "falling", debounce: 70});

It's not obvious at all that writing to the console requires that console to be connected all the time. The same way when I want to communicate something to the user by flashing a red LED then I simply set the LED's pin and I'm done, doesn't matter if the LED is actually connected. When I want to write something on the LCD then it wouldn't freeze if it's not connected etc.

Sorry but I think this is nasty and counter-intuitive. Also how about the device modules that are happily using console.log (quite a few)?

@gfwilliams
Copy link
Member

IMO what's more counter intuitive is if what you print to the console gets completely lost if the connected PC doesn't read it in a timely manner. For instance:

  • maybe you print a big string and the PC is slow at reading it (maybe it's a script that tries to connect to the net after receiving every line). Suddenly it doesn't read a value in 2 seconds and all the data you printed gets lost.
  • maybe you use console.log and then read the data with a cron job. That's a pretty reasonable thing to want to do, but changing this behaviour makes it impossible.

Actually losing data when the device is connected to USB is a real problem - but this is just documented behaviour.

how about the device modules that are happily using console.log

Which ones? The only one that I know of is the NRF24 module - and iirc that only happens on errors or if you haven't specified a callback function.

I'm open to suggested solutions that solve this but don't randomly throw away data... However this is all documented in the troubleshooting page, so it's not rocket science to look it up and see what's wrong.

@bartmichu
Copy link
Contributor

Yes, if you put it this way - either freeze the whole thing when output device is not connected or randomly loose data when it is - then it suddenly looks like a feature. Should these two be tied together? Don't think so. It's a while since I was playing with Arduino but I don't remember it was dying on me when the serial console was not connected.

Which ones?

AT24, BMP085, Midi, Pixy, PN532, STM32F1Flash, Touchscreen, XBee-API1, NRF24LO1P, PN532

@gfwilliams
Copy link
Member

No - it probably randomly lost data when the USB terminal wasn't connected :) Might be worth a try - you could see what happens with a USB-Serial converter as that's what's inside most Arduinos.

Eep. Thanks - I'll have to fix that (although I'm sure some of those prints only happen on errors?)

@bartmichu
Copy link
Contributor

although I'm sure some of those prints only happen on errors?

To be honest I didn't analyze them, it was just a quick search as I needed as many examples as possible to prove my point ;) You are right, it's mostly when they try and communicate errors. Some of these errors are programmer's fault (i.e. wrong parameters) but some of them are just things that can happen with hardware e.g. wrong checksum for whatever reason.

@loop23
Copy link

loop23 commented Apr 23, 2014

Well of course freezing when it can't print is not to be considered a feature IMHO. Btw, is there a semantic difference between console.log and print? that is, which should be used when? Maybe make print hang (it's meant to be read) and console.log not? (as it's meant for debugging) I actually just realized my minimodule does both, but for similar reasons, so which should I pick?

@gfwilliams
Copy link
Member

Well of course freezing when it can't print is not to be considered a feature IMHO

Well, it's not a bug.

It's done that way for a reason, and the behaviour is documented. If you do cat myfile | more on the desktop, you don't expect cat to throw away the file contents because you're not constantly requesting data.

console.log and print are absolutely the same function. I'd be tempted to use console.log so it's more compatible with desktop JS though.

Honestly, if I changed them then people would still use the wrong one and complain here. I don't think it would help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants