node.flashreload() Not Working

[chathurangawijetunge]

NodeMCU 3.0.0.0 built on nodemcu-build.com provided by frightanic.com
	branch: dev
	commit: 0e02c0e5f30f9944d70a58eb97c6cd8e4292be2b
	release: 
	release DTS: 202008232211
	SSL: false
	build type: integer
	LFS: 0x40000 bytes total capacity
	modules: file,gpio,mqtt,net,node,rtctime,sntp,tmr,uart,wifi
 build 2020-08-26 02:44 powered by Lua 5.1.4 on SDK 3.0.1-dev(fce080e)
cannot open init.lua: 
----------------------------------------
node.flashreload("os.img")
Lua error: 	stdin:1: attempt to call field 'flashreload' (a nil value)
stack traceback:
	stdin:1: in main chunk
	[C]: ?
	[C]: ?

[KT819GM]

There was changes introduced. Do you use Gitter? If yes, please check it. If not:
'node.LFS.reload('lfs.img')'
'LFS=node.LFS'
'LFS.module()'

[chathurangawijetunge]

There was changes introduced. Do you use Gitter? If yes, please check it. If not:
'node.LFS.reload('lfs.img')'
'LFS=node.LFS'
'LFS.module()'

Ok... this works but what about all the other functions related to old method like...... node.flashindex() etc.
and what functionality does 'get' do (as its allays nil)

for k, v in pairs(node.LFS) do print(k,v) end
list function: 402492d8
get function: 40248478
reload function: 4024858c

[marcelstoer]

https://nodemcu.readthedocs.io/en/latest/modules/node/#nodelfs

[chathurangawijetunge]

https://nodemcu.readthedocs.io/en/latest/modules/node/#nodelfs

thanks

[HHHartmann]

I had the impression that we wanted to deprecate this which should add a deprecation warning, printed on usage. So this should work as it used to.
See #3176 (comment)

[TerryE]

It should work for backwards compatibility, I don't mind having a deprecation warning in the documentation, but when did we agree that any deprecated function should print a warning on usage. This can change the behaviour of a legacy app. Before we adopt this as a policy, I feel that we should debate and vote on this.

[TerryE]

NodeMCU 3.0.0.0 
	branch: dev
	commit: 606f91664b324c45be44313434a34b3c6e8ec766
	release: 3.0-master_20200610 +20
	release DTS: 202008291648
	SSL: false
	build type: float
	LFS: 0x0 bytes total capacity
	modules: adc,bit,dht,file,gpio,i2c,mqtt,net,node,ow,spi,tmr,uart,wifi
 build 2020-08-29 18:27 powered by Lua 5.1.4 on SDK 3.0.1-dev(fce080e)
cannot open init.lua: 
> =node.flashindex
function: 0x402495fc

I would prefer to have the "Do all deprecated uses print a usage warning as a separate policy issue. Gregor, feel free to open this if you want the discussion.

[marcelstoer]

when did we agree that any deprecated function should print a warning on usage

We have been doing this for years through a dedicated function in platform.c:

$ grep -R --include=*.c platform_print_deprecation_note *
modules/ws2812_effects.c:  platform_print_deprecation_note("ws2812_effects",
modules/mqtt.c:      platform_print_deprecation_note("mqtt.connect secure parameter as integer","in the future");
modules/tls.c:    platform_print_deprecation_note("tls.cert.auth's old interface", "soon");
modules/tls.c:    platform_print_deprecation_note("tls.cert.verify's old interface", "soon");
modules/crypto.c:  platform_print_deprecation_note("crypto.toBase64", "in the next version");
modules/crypto.c:  platform_print_deprecation_note("crypto.toHex", "in the next version");
modules/crypto.c:  platform_print_deprecation_note("crypto.toBase64", "in the next version");
modules/crypto.c:  platform_print_deprecation_note("crypto.toHex", "in the next version");
modules/node.c:      platform_print_deprecation_note("node.info() without parameter", "in the next version");
platform/platform.c:void* platform_print_deprecation_note( const char *msg, const char *time_frame)

We first add add a call to platform_print_deprecation_note in addition to the existing code in the function we want to deprecate. At the same time we add a deprecation note to the docs. A couple of releases later we then drop the function - unless we forget this 2nd step.

[TerryE]

I know that, but there is a difference between a facility that may be used when appropriate and one that is mandatory. An LFS app might do 10s or even 100s of index lookups per sec. Adding this deprecation note could cause major timing problems and even timeout to a working app. The old none-object forms of timer and file were similarly deprecated. Adding a mandatory deprecation note to these would be similar insanity.

[HHHartmann]

@TerryE Terry, true that flashindex may be called quite often, but flashreload usually will not be called that often.
And we agreed to leave it in for a deprecation time.

So I see this issue as valid.

[TerryE]

OK, I will add the deprecation warning to flashreload on the next set of updates.

[HHHartmann]

The problem is not only the deprecation warning, but that you seem to have removed the flashreload command entirely.
We should add it back in soon, but definitely before the next master drop.
@TerryE Will your next set of updates be before or after the next drop?

[TerryE]

@HHHartmann, not sure how this happened. It's a 1-liner to put back.

[HHHartmann]

Ok. I can do it, just didn't want to cause merge trouble for you

[marcelstoer]

not sure how this happened

a92da3c#diff-dd307274a140ac914b5e66af503869cfL829

[TerryE]

@marcelstoer I know when it happened. That was a big change: 75 files changed with 1,644 additions and 776 deletions. The new line LROT_TABENTRY( LFS, node_lfs ) at line node.c:874 was supposed to be inserted and not replace the LROT_FUNCENTRY( flashreload, lua_lfsreload ) line; a simple edit error that disabled node.flashreload(). My typing an extra ^D at the wrong time in the editor is how it happened. BTW, the discussion was here on #3176.

[TerryE]

Ok. I can do it, just didn't want to cause merge trouble for you

@HHHartmann Gregor, on reflection, I will add this to #3272 because I also want to add #3145 at the same time. We need this for on ESP LFS maintenance.

[marcelstoer]

Gregor wanted to have this fixed before the next master drop. I support that.

[TerryE]

Done. See 41c5f8f

[marcelstoer]

The good thing about PRs is that we sometimes spot issues before they land 😜
-> https://travis-ci.org/github/nodemcu/nodemcu-firmware/jobs/724897202

[TerryE]

Marcel, OK. Thanks for pointing this out, but fixing it would have just been as quick. I've now corrected this, d88c14b, and I will do a two stage PR / merge in future.