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

add build_date to build info #2888

Merged
merged 1 commit into from
Aug 15, 2019
Merged

Conversation

HHHartmann
Copy link
Member

Fixes #2830 (comment).

Make sure all boxes are checked (add x inside the brackets) when you submit your contribution, remove this sentence before doing so.

  • 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.

Add automatic BUILD_DATE to Build Info

@marcelstoer marcelstoer added this to the Next release milestone Aug 15, 2019
@marcelstoer marcelstoer merged commit 739b675 into nodemcu:dev Aug 15, 2019
@TerryE
Copy link
Collaborator

TerryE commented Sep 13, 2019

Gregor,

I've belatedly gone through this PR. One my current build this gives me this reboot prologue:

NodeMCU 3.0.0.0 
	branch: dev-lua53
	commit: 42cbd1aa2cf503759956bbee5dbea422c76cdd40
	release: 2.0.0-master_20170202 +449
	release DTS: 201909071447
	SSL: false
	build type: float
	LFS: 0x0
	modules: adc,bit,dht,file,gpio,i2c,mqtt,net,node,ow,spi,tmr,uart,wifi
 build 2019-09-07 15:47 powered by Lua 5.3.5 on SDK 3.0.1-dev(fce080e)
>       

I can see the significant advantage of having this sort of summary for the basic developers because this is the sort of info that we need them to provide when raising issues, but this is also a PITA for advanced used and in particular those using deep sleep.

  • At a minimum this long format should be suppressible through a user_config define.
  • NODE_VERSION should be a subset of NODE_VERSION_LONG and not v.v.
  • The Release and Release DTS fields are redundant, and misleading in the case of dev builds
  • The script update_buildinfo.sh will die miserably if developers do their own make based on a ZIP download. You have added undocumented and sometimes incorrect assumptions in the implementation.

Note that to reduce startup times I am considering deferring printing the NODE_VERSION until after execution of LUA_INIT_STRING and also parsing the return from its execution (i) optionally to suppress this print and (ii) optionally to suppress startup of the interactive poll, but that's a separate PR.

@marcelstoer
Copy link
Member

Terry, I can suggest we discuss this in a new issue rather than in a closed PR? Maybe you could weave it into #2909?

this is also a PITA for advanced used and in particular those using deep sleep

Why is that so? And how is it related to deep sleep specifically?

@HHHartmann
Copy link
Member Author

NodeMCU 3.0.0.0 
	branch: dev-lua53
	commit: 42cbd1aa2cf503759956bbee5dbea422c76cdd40
	release: 2.0.0-master_20170202 +449
	release DTS: 201909071447
	SSL: false
	build type: float
	LFS: 0x0
	modules: adc,bit,dht,file,gpio,i2c,mqtt,net,node,ow,spi,tmr,uart,wifi
 build 2019-09-07 15:47 powered by Lua 5.3.5 on SDK 3.0.1-dev(fce080e)
>       

I can see the significant advantage of having this sort of summary for the basic developers because this is the sort of info that we need them to provide when raising issues, but this is also a PITA for advanced used and in particular those using deep sleep.

I agree

  • At a minimum this long format should be suppressible through a user_config define.
  • NODE_VERSION should be a subset of NODE_VERSION_LONG and not v.v.

True, My aim was not to break any scripts automatically changing this. May be finding another name for NODE_VERSION_LONG would be a googd idea as well. How about BUILD_INFO_BANNER or whatever.

  • The Release and Release DTS fields are redundant, and misleading in the case of dev builds

Release DTS holds the date of the commit, so it probably should be renamed to Commit DTS.

  • The script update_buildinfo.sh will die miserably if developers do their own make based on a ZIP download. You have added undocumented and sometimes incorrect assumptions in the implementation.

Ok, indeed. Seems as nobody thought of that. But the script does not fail the build, it just fills the variables with empty values.

Note that to reduce startup times I am considering deferring printing the NODE_VERSION until after execution of LUA_INIT_STRING and also parsing the return from its execution (i) optionally to suppress this print and (ii) optionally to suppress startup of the interactive poll, but that's a separate PR.

That sounds like a good idea.

But could we discuss this in an open issue? maybe #2909 or a new one?

@TerryE
Copy link
Collaborator

TerryE commented Sep 13, 2019

Marcel, first of all sorry for not picking up #2909, and I agree that this is the correct home for these discussions and I see that you have already made many of my points.

Why is that so? And how is it related to deep sleep specifically?

Unwanted output to the Tx pin can be a bit of an issue. SDK 3.0 already produces enough and this just makes it worse.

As to deep sleep, as far as Lua startup is concerned this is is just a restart so you'll just get this all repeated every wake.

@HHHartmann HHHartmann deleted the add-build-date branch May 1, 2020 16:22
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

Successfully merging this pull request may close these issues.

3 participants