-
Notifications
You must be signed in to change notification settings - Fork 90
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
slowWrite is automagically configured, and is not logged even with -v verbose flag and the default is true #178
Comments
there was one more gotcha, currently there is no semicolon sent at the end of write(), this expression matches now and splitting storage writes works split = findSplitIdx(split, /\x10[^\n]*require\("Storage"\).write\([^\n]*\)[;]*\n/, 500, "Storage.write"); // Write chunk of data EDIT: created PR for this #179 |
This is very frustrating - I've recently hit this with serial on STM32 as well. Thanks for #179 - I'll have a look at a fix for the line numbers, because it's pointless and will slow things down too. I forgot it's a default as I think I turn it off here.
But perhaps we could improve the description of this so it's easier to find?
I guess we could do that - ideally in EspruinoTools/plugins/versionChecker.js Lines 59 to 66 in 8eb606c
SERIAL_THROTTLE_SEND=2 though?
Not unless there's another option, but it's just a setting that can be changed (see above). Basically if upload is a bit slow usually nobody really notices, but if it breaks I get all kinds of complaints from everyone and it makes Espruino look unreliable. Which is why the slow write is on by default unless I'm sure it'll work on a device. As a bit of background: Normally on serial it wouldn't be an issue as Espruino's default baud rate was 9600 baud, but then when the port to ESP8266 was created the contributors chose 115200 baud because, you know, speed. But then it was really unreliable and nobody would change back to 9600 baud so I had to add this whole slow write hack in the IDE just to work around it and bring the speed back down to about 9600 baud :( I hit this recently, and for me, with USB-TTL connections (and I guess with the TCP connection too) I believe there's quite a large output buffer on the PC. So when the board sends back XOFF there's still several thousand chars en-route still, which then fills up the buffer even when it's at ~2k or more. As far as I can tell there's not really any way around that from Web Serial apart from throttling to reduce the amount of chars en route. That ESP32 line does look broken, but it may be just as well because I think ESP32 would still break if it was enabled, even with flow control (because of the above): EspruinoTools/plugins/versionChecker.js Line 65 in 8eb606c
Yes, I imagine that is fine as hopefully TCP will have the flow control built in anyway so we're not worried about losing things... because XON/XOFF 100% won't work given TCP's lag and buffering. So really, we're a bit stuck. Definitely, we should be outputting more info in the log about when slow write is enabled, and the config option needed to disable it (but for most people, disabling slow write will just break things). The delay on storage write will help but still if Storage has to be compacted that'll add a delay and the upload will fail. I feel like slow write is a hack, and there are two main 'proper' solutions, both of which are a bit similar:
|
To be fair, I just tried and logging does say quite clearly:
I'm not sure it should also spam you with 100s of lines saying it's enabled. |
For me the
and there it is turned off by default and still it was slow. Now I updated to latest npm version 0.1.56 and it changes to list of values like you say but again, it does not affect the slowWrite variable which is true and stays true and it is slow even with
says "Command-line option set Espruino.Config.SERIAL_THROTTLE_SEND to 2" but still splits into 19 chars every 50ms. BTW where do I see the "Set Slow Write = true ('Throttle Send'='Auto')" log you mentioned? Don't see that on screen with "-v". And the --listconfigs says just
|
I was hoping that would work with openocd telnet port vs espruino command -p tcp://127.0.0.1:2222 but either there is still some bug with XON/XOFF and SWDCON/RTT or you are right. I reduced buffer to 64 chars https://github.com/fanoush/openocd/blob/f-rtt-server-write-retry/src/server/rtt_server.c#L32
Yes, any of those two would be much better solution, lot of data is going out and nothing back to confirm what happens. The OK looks easier/simpler. If you would define some temporary method to do the write + OK response then the uploaded code could be even slighly smaller due to the |
I just checked and what happened was in the IDE we called setSlowWrite(true) when starting a new connection (which we should do) and then when we figure out what we're connected to we see if we can disable it. So if you connect using the IDE you see the message. But in the CLI we never called setSlowWrite(true) initially so you only see SetSlowWrite = false on a board it trusts. I believe I've fixed that now if you use the latest from GitHub, and I added a message too
Yes, I had considered that for Bangle.js too. For now it's probably safer not to try |
Yes, thanks, the b9c0f37 fixed it, now I can override with --config SERIAL_THROTTLE_SEND=2 and it actually works. About the OK message, maybe it doesn't need to be after every line but just after last one? What happens for me with SWDCON is that it just takes time but all writes are done correctly even when sent in one big batch so only if after last write there was some |
Was trying to figure out why uploading file to storage takes many minutes and had finally look into source.
espruino --help
,espruino --listconfigs
has no hint there is something likeslowWrite
flag-v
logs a lot of stuff including data that is going to be sent but again nothing that would hint what happens while the data is transferredFinally I had to find and uncomment this line
EspruinoTools/core/serial.js
Line 386 in 8eb606c
Maybe this line could be enabled with "-v"?
Some magic is here
EspruinoTools/plugins/versionChecker.js
Lines 59 to 66 in 8eb606c
EspruinoTools/core/serial.js
Line 47 in 8eb606c
was silently used with no logging. BTW why the ESP32 message says it uses flow control and then slow writes are set to true, shouldn't it be false because flow control works and can stop the flow?
I was actually using
-p tcp://127.0.0.1:2222
so also the driver inhttps://github.com/espruino/EspruinoTools/blob/8eb606cff5992a64c3127b8bf32fd0b01ad7873c/core/serial_node_socket.js
has no workaround like e.g. this
EspruinoTools/core/serial_web_bluetooth.js
Line 192 in 8eb606c
more places https://github.com/search?q=repo%3Aespruino%2FEspruinoTools%20setSlowWrite&type=code
And looks like the versionChecker.js is parsing console name from
process.env
(USB, Telnet, ...) instead of setting it from the real transport that is used. So in my case I was using TCP port but my console was not named "Telnet". I am not sure adding "SWDCON" next to "Telnet" is good workaround.Another puzzling thing was that I found this code
EspruinoTools/core/serial.js
Lines 354 to 358 in 8eb606c
that contains matches for Storage.write with some 500ms delays but it does not match because default is "--config STORE_LINE_NUMBERS=true" and there is extra stuff after "x10" with line number - like
u0010\u001b[10drequire(\"Storage\").write
. When fixed it does not match anyway (maybe because ofEspruinoTools/core/serial.js
Line 338 in 8eb606c
Some suggestions to improve this:
Can the slowWrite be false by default?
If not can the serial_node_socket set it to false like in other place and Telnet is removed from the check?
Can the slowWrite flag be overriden from command line or --config parameter so it is more visible?
The text was updated successfully, but these errors were encountered: