Extend node.info

[HHHartmann]

Fixes #2799 #2398 #1739

• This PR is for the dev branch rather than for master.
• This PR is compliant with the other contributing guidelines as well (if not, please describe why).
• I have thoroughly tested my contribution.
• The code changes are reflected in the documentation at docs/*.

There are different variations of the firmware resulting from different modules, branches, SSL, int/float, LFS and release.
To better identify these firmware builds these information are made available in node.info() and in the startup message.

Currently I am having a problem with generating the commit DTS. It does not work for the docker image and I can't check it under other linux.
Works on my Win10 though:
git show -s --format=%cd --date=format:"%Y%m%d%H%M" HEAD generates 201907121923 which corresponds to the localtime of the commit which was at Fri Jul 12 19:23:02 2019 +0200.
On docker the --date format is not recognized. In fact no format seems to be known at all there.
But even on win10 it should be UTC and not localtime to give an order.
So any help/suggestions welcome here.

Still needs the documentation but is short it adds the following defines to node.info()

BUILDINFO_BRANCH, BUILDINFO_COMMIT_ID, BUILDINFO_RELEASE, BUILDINFO_RELEASE_DTS, BUILDINFO_SSL, BUILDINFO_LFS, BUILDINFO_MODULES, BUILDINFO_BUILD_TYPE.

It also adds a string similar to that generated by docker (from which I took most of the code to generate the defines) to the welcome message.

Any comments welcome!

[HHHartmann]

OK messed it up. Sorry

[marcelstoer]

It would be nice if a build tool (cloud builder, Docker) could add its vanity token into the build info. Actual tokens would include something along the lines of "built by my super build tool".

[HHHartmann]

Forgot to mention that the environment variable USER_PROLOG can be set to add a vanity token.

[marcelstoer]

But even on win10 it should be UTC and not localtime to give an order.

TZ=UTC git show --quiet --date=format-local:"%Y%m%d%H%M" --format="%cd" works for me on the Mac: 201907121859

[HHHartmann]

Build is broken. Will have a lock.

[marcelstoer]

Build is broken.

Yup, I left a comment but that one got somehow lost: make: execvp: tools/update_buildinfo.sh: Permission denied

[HHHartmann]

Also deliberately broke replacement of NODE_VERSION by the docker script by inserting a space after the # in that line.

[marcelstoer]

On docker the --date format is not recognized. In fact no format seems to be known at all there.

That is caused by an old Git version.

My Mac: git version 2.20.1 (Apple Git-117)
Docker container: git version 1.9.1

The --date=format-local: feature is new in Git version 2.7.0. On Ubuntu Trusty you won't get beyond 1.9.1 without a work-around. However, we/I should be upgrading to Xenial anyhow because that's what we're using on Travis.

[HHHartmann]

So what do we do about the DTS and old git version.
Wait for an updated docker image or find another solution?
Initially we pinned docker to trusty because of marcelstoer/docker-nodemcu-build#39

[marcelstoer]

I started with the image update over at marcelstoer/docker-nodemcu-build#69. Feel free to work with this. ESP8266 seems fine but ESP32 still fails. Not sure when I can work on that next time.

[nwf]

Sorry to chime in late, but the list of returns seems like it's... lengthy. Would it make sense to group some or all of them into (a) table(s)?

[HHHartmann]

To have a consistent interface I would not like to touch the existing interface.
It would also feel odd to deliver half of the information one way and the other in another way.

I can think of these ways to go:

1. Add a new 'node.buildinfo()' which returns { branch = "Extend-node.info", ... }.
   But then the major/minor/dev-Ver stuff would belong there too. Maybe duplicate it.
2. Add the old and new information in an array as last param in the list of node.info()
3. Break the interface and just retrun an array with all information
4. just add the new values to the list

So I tend to use 1. or 4. unless a breaking change would be OK. Then 3. clearly would be best.

[nwf]

I think 3 violates the standards we try to hold ourselves to, of giving a deprecation warning and all that.
I think, perhaps, deprecating node.info() and having one or several replacement calls to provide the information in a more sensible arrangement of immediates and within tables is perhaps a good idea.

How about this:

• node.info() or node.info(nil) behaves as now, i.e., returns no new information, and throws a deprecation warning.
• node.info("hw") returns chipid, flashid, flash size, flash mode, flash speed.
• node.info("sw_version") returns version, branch, git commit_id, release, release_dts.
• node.info("build_parameters") returns number_type ("integer" or "float"), ssl (boolean), and lfs (nil if disabled or integer size in bytes [please don't use "Size: %d" style replies]).

Whether those results should be direct results or entries in a table, I leave to you. Given such a "topic"-ized form of node.info, it's a little less worrying that we're going to end up returning dozens of values at once.

[HHHartmann]

Just now read your comment.
I like your suggestion. Maybe even better call it "build_config".
I am not sure about the return value. Having a list like now allows print(node.info("hw")) for getting an overview on the console. But having an associative array allows if node.info("build_config").lfs which generates better readable of code.
OK thinking about it, readability wins.

[HHHartmann]

@TerryE Terry would you please care to have a close look at the last commit here? Would like to know if the c part is ok like that.

[marcelstoer]

@nwf I like your proposal, makes a lot of sense to me. How about support for info("all") or making the parameter a vararg so I could say node.info("hw", "sw_version")?

[TerryE]

Cutting across a number of PRs and issues is this whole discussion of best practice for writing NodeMCU libraries, e.g. Jonhy and I were discussing the whole issue of Lua stack balancing for CBs and we've talked about other helper wrappers around the luaL_ref / unref stuff. I think that we should take this as a separate standalone issue.

[HHHartmann]

NOTE: DTS calculation requires git version 2. something. Currently the docker image has Version 1.9.
So in any old git version the DTS will just be empty. Given that this PR even then offers some improvement I suggest to not wait for the updated docker image to merge it.

[nwf]

You know, I hate to do this to you again (thank you for the rewrite after my last suggestion), but... thinking about your node.info("build_config").lfs example above... would a yet-better API be for us to add a node.info_table that's a rotable whose __index metamethod is a C callback that returns the field being asked for? That way node.info_table.lfs returns the right thing and we don't push all the strings into Lua's heap at once, just on demand.

@TerryE, is such a thing reasonable from the Lua side?

[HHHartmann]

I didn't like all those char* and creating Lua objects all the time.
If using __index also enumeration would not work if I understand this thing correctly (no deeper understanding on my side (yet))
I guess one could also add an enumerator but I would expect this functionality to not be used too often, so maximum optimization is probable not needed here. Maybe it would make sense though adding the keys to the LFS dummy_strings.lua.

[TerryE]

Nathaniel, I suggest that we are pragmatic here. Returning the array keeps the implementation simple. In practice the caller will cherry pick the fields that are wanted then ditch the array.

[nwf]

This might belong in a separate PR, but I'm not going to complain too much.

[nwf]

Fair enough. In that case, I think I am happy with the topic-ized version of node.info and this PR.

[marcelstoer]

I'm really happy how this turned out. Very nice improvement Gregor. Thanks!

[nwf]

This seems to be the last PR open for the next release milestone. Is there anything left blocking this landing?

[HHHartmann]

Not that I know of.

[marcelstoer]

Oh, I only now realized that update_buildinfo.sh doesn't set the build date. Did you omit this on purpose? Wouldn't it be useful to have BUILD_DATE="$(date "+%Y-%m-%d %H:%M")" in there?

[marcelstoer]

@HHHartmann However, we'd have to invert that order of precedence.

If we do

BUILD_DATE="$(date "+%Y-%m-%d %H:%M")"
#define BUILD_DATE "$BUILD_DATE"

in the script the #ifndef BUILD_DATE would guarantee that any date the user sets explicitly is overwritten. It should be the other way around.

[HHHartmann]

We could set BUILDINFO_BUILD_DATE and then be able to get it right in user_version.h

[marcelstoer]

Yep, that sounds like a good plan.

[johndoe71rus]

excuse me. Please advise what is wrong.
https://paste.ubuntu.com/p/dMpZXdb5ZR/

On version 2.0, work code
espinfo = {} espinfo.majorVer, espinfo.minorVer, espinfo.devVer, espinfo.chipid, espinfo.flashid = node.info()
how now to collect information about the module?

[marcelstoer]

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

[nwf]

@johndoe71rus Please don't perform necromancy of PRs as a support channel. That said...

The code you have quoted here (which calls node.info()) and the code in your paste link (which calls node.info always with a string argument) are different. The code you have quoted here will work with the current release (but is deprecated), and so I'm not sure why you've included it; is something wrong with that particular invocation? I suggest you review the documentation (https://nodemcu.readthedocs.io/en/master/modules/node/#nodeinfo).

To be a little more precise about what seems to be ailing you, tho': if you look at your paste's printout, you will see that your espinfo.git_branch is being set to table: 0x3fff00d8. That table contains keys like git_branch, so try espinfo = node.info("sw_version") instead.

[johndoe71rus]

before this changes i could collect sw and hw info, use wrote code above.
now i try collect all info in one espinfo object, but node.info("sw_version") and node.info("hw") rewrite whole espinfo.
https://paste.ubuntu.com/p/JwpCzGZ469/
I agree that node.info("hw", "sw_version") could help me. But this not released yet.

[nwf]

This is not a suitable forum for the kind of support you are requesting. Please see https://nodemcu.readthedocs.io/en/master/support/ .