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

node.info() should return better information about the actual NodeMCU version #2398

Closed
joysfera opened this issue Jun 14, 2018 · 13 comments
Closed
Assignees

Comments

@joysfera
Copy link
Contributor

Expected behavior

node.info() returns something concrete that identifies the version properly. At the very least the devVer would increment on each master release. Or a new releaseDate would be added to the node.info() table that would contain 20180608, a number derived from date that would increase on each release.

Actual behavior

node.info() returns 2, 2, 0 in its Ver fields for both the April and June releases, even though there were important bugfixes and even API changes in the latter.

I wish a Lua application was able to identify the NodeMCU build precisely enough so I'd know which nodes need updating the NodeMCU firmware when a new release with an important bugfix gets available.

Test code

=node.info()

NodeMCU version

2.2.0-master_20180608

Hardware

ESP-07

@TerryE
Copy link
Collaborator

TerryE commented Jun 14, 2018

We already edit user_config.h in our Travis script. Maybe we should add $(git rev-parse HEAD| cut -b 1-8) to the version string or even just as a -D option in the app/lua Makefile.

@joysfera
Copy link
Contributor Author

I'm not sure if git commit ID is a number that could be compared with another number to find out whether a node is running an outdated NodeMCU or not. That's why I suggested the 20180608 number (currently part of the release "name") that is guaranteed to be increasing on each release.

Granted, it wouldn't identify git builds, but it would work for released ones.

Maybe we'd need both things - the exact commit of the build and also a version number that does increase on release.

@TerryE
Copy link
Collaborator

TerryE commented Jun 14, 2018

All releases are published on the basis of a git commit, so any release has a unique git HEAD. What you seem to be wanting is an ordered value rather than a unique one. What about the DTS of the commit:

git show -s --format=%ci HEAD

@joysfera
Copy link
Contributor Author

Well, that would work (more or less) if turned into an integer YYYYMMDD. I realize it doesn't allow for two releases in one day but I don't think that's an issue given that releases are at least two months apart.

But I'm surprised that we have majorVer, minorVer and devVer - seems almost like https://semver.org/ but it's not used that way. Any reason why?

@marcelstoer
Copy link
Member

But I'm surprised that we have majorVer, minorVer and devVer - seems almost like https://semver.org/ but it's not used that way. Any reason why?

I didn't check the code but IIRC those values represent the SDK version.

@joysfera
Copy link
Contributor Author

@marcelstoer just a small FYI: the NodeMCU build prints that it's based on SDK 2.2.1 (since April) but the devVer still contains 0 so even if these *Ver fields were representing the SDK they don't match with the actual information shown at boot.

And if they indeed represent the SDK (which is not documented so IIRC) then we definitely need a new version field that will represent the NodeMCU itself.

@TerryE
Copy link
Collaborator

TerryE commented Jun 15, 2018

If we are going to embed a unique identifier, then the solution has to work for dev builds as well as master which is why we should include either the git commit ID or the git commit DTS.

@joysfera
Copy link
Contributor Author

I understand the need of unique ID based on git but if the releases are going to be marked by YYYYMMDD (like they're now, and I like it much) then I'd like to also have this "release ID" accessible from within the Lua program, because it's immediately obvious how old the firmware is and it's easy to find out what's missing or wrong there.

So maybe we're aiming at two new fields in node.info(): a gitID and a releaseVer.
If the NodeMCU releases switched to semantic versioning sometime in future then of course the releaseVer would have to be split to major, minor and patch fields as well.

@TerryE
Copy link
Collaborator

TerryE commented Jun 16, 2018

One reason is that any solution that we use has to work for both dev and master. We don't necessarily need to use the git ID, we could have a releaseDTS which is of the format YYYYMMDD-HHMM based on the DTS of the git commit, which in practice is both unique and ordered so the > operator is meaningful.

@karrots
Copy link
Contributor

karrots commented Jun 16, 2018

One reason to leave the git ID in there somewhere is folks are building using the cloud builder on dev and master. It would be good to know when they submit trouble tickets which build they were built off of.

@marcelstoer
Copy link
Member

We shouldn't be arguing about whether field X should be part of info or not - more information is always better than less, isn't it? So, any information we can somehow automatically embed into the firmware is helpful. For as long as we stick to our current regime where the Git tag name kind of represents the NodeMCU version we can:

  • add the SDK version (already got that but possibly buggy)
  • add the result of git tag -l --points-at HEAD
  • add the result of git rev-parse HEAD

@joysfera
Copy link
Contributor Author

I agree with @marcelstoer because I realized that the DTS (@TerryE 's idea) does not carry the information about branch, and since dev can differ from master a lot the YYMMDD-HHMM mark would be ambiguous...

@HHHartmann
Copy link
Member

Should be fixed with #2830

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

No branches or pull requests

5 participants