NodeMCU-Tool icon indicating copy to clipboard operation
NodeMCU-Tool copied to clipboard

JS library sometimes uploads files with 0 bytes

Open czenker opened this issue 3 years ago • 5 comments

Environment

  • Operating System
    Fedora 33
  • ESP Device/Revision
    ESP8266
  • NodeMCU-Tool Version
    Please use the least recent version to validate the issue
    3.2.1
  • Node.js Version
    14.15.4
  • NodeMCU LUA Firmware Version
    3.0.0

Issue Description

When using NodeMCU-Tool as library for JS, the upload function will create files with 0 byte size if any of the following conditions is met:

  • The device is reset between file uploads
  • files are uploaded to two different connected devices

Expected Behavior

Files are created on the board...

Current Behavior

... in some cases they have a size of 0 bytes.

Steps to Reproduce

  1. create example.txt with a non empty content
  2. write a JS program that does the following
    1. connect to a device and upload a file
    2. reset the device
    3. connect to the same device and upload a file
  3. the second file will have a file size of 0 bytes

Example

const nodemcu = require('nodemcu-tool')

const dev = "/dev/ttyUSB0";

(async () => {
  try {
    await nodemcu.connect(dev)
    await nodemcu.upload("./example.txt", "example.txt", {}, () => {})

    console.log(await nodemcu.fsinfo())

    await nodemcu.hardreset()
    await nodemcu.disconnect()

    await new Promise(r => setTimeout(r, 2000)) // wait for reboot

    await nodemcu.connect(dev)

    await nodemcu.upload("./example.txt", "example.txt", {}, () => {})
    console.log(await nodemcu.fsinfo())
  } catch (e) {
    console.error(e)
  }
})()

Detailed Description

Core problem is that _isTransferWriteHelperUploaded is tracked as a global variable and is never reset. It should handle device changes, reboots, disconnections, etc.

https://github.com/AndiDittrich/NodeMCU-Tool/blob/f160e0abbbe89927fbdc8c6a308b4d2713009fc8/lib/connector/upload.js#L9

Side note: The download function does not show this behavior.

Possible Solution

Remove the check, if transferWriteHelper was written and write it before every single file upload.

An alternative approach is to reset _isTransferWriteHelperUploaded whenever a connection to a new device is established or it is reset. But this would create dependencies between unrelated modules. And it would not cover the case when the user or a script issues a reset through the execute function.

A different alternative is to check if _G.__nmtwrite exists on NodeMCU before every write. But I would disregard that, because if we need to execute a lua script, we might as well just write the transferWriteHelper.

Pull-request inbound.

czenker avatar Feb 14 '21 11:02 czenker

HI @czenker

thanks for your report.

a reset during connection is not projected to be handled by the library.

imho the only reliable solution is to wrap the file transfer helper within an instance of the current connection but this require stateful components within the library.

at the moment i would mark it as bug and wont-fix since the use-case is very limited

AndiDittrich avatar Feb 14 '21 11:02 AndiDittrich

Hi @AndiDittrich.

Thanks for the fast response. I think the main issue here is that it is very easy to get nodemcu-tool (the JS library) into a state where the upload does not work. If there was a way to programatically recover from it, I would not consider it a too big issue. But it is not possible to recover from that state without restarting the NodeJS(!) application.

czenker avatar Feb 16 '21 16:02 czenker

ok. the underlying library was never intended to be used as regular library.

AndiDittrich avatar Feb 16 '21 17:02 AndiDittrich

Alright. This decision makes sense then.

ProgrammaticUsage.md should be updated, because it states

It's possible to use the underlying "NodeMcuConnector" in your own Node.js projects

czenker avatar Feb 16 '21 18:02 czenker

thanks, i've added a usage notice

AndiDittrich avatar Feb 17 '21 09:02 AndiDittrich