Allow increment after successful upload + tidyup

[pfeerick]

For me, this resolves #3. Please kick the tyres, see what you think, and if you like it, accept it. After some brief testing, it seems to be working for me so far, but it's early hours ;)

Summary of changes include

• create and use include folder for header if missing, as not doing so resulted in a Version.h not found error (since project root is not in include path)
• uses pre-scripting to create version files in pre-build, so the first compile will be successful
• uses post-scripting to increment the version number post-upload, which seems to only run if PIO doesn't know about an upload failure
• variable names, trailing whitespace, some function call changes since I made the mistake of running pylint :-O

[sblantipodi]

Many many many thanks for your work pfeerick, I like it and I am going to accept it, well made contribution are always accepted :)

I tought on this before I closed #3 but I discarded the solution due to the fact that in this way I will commit a N+1 version on my VCS while my devices are running the N version.

Basically the version and Version.h header will always have a N+1 version than what real devices are running.

how can we workaround this problem?

[pfeerick]

Aw... shucks... you spotted the one fly in the ointment! 🤣

Have to think about that one a little more. I was going to do a .gitignore on the include/version.h since that is auto-generated, and keeps changing... leaving it to the VERSION file to track the version. But that still leaves the N+1 issue.

I almost want to do a N+1 in the pre-script also, meaning the post script effectively writes the current version, rather than the next. Maybe this is could work if the 'version doesn't exist hence start from 0` instance is taken care of better... ☕ 🍫

[sblantipodi]

If you do some math...

• In this way the version number is 100% wrong.
• If you do N+1 in the pre script, the version number is wrong only in the case of an upload failure but it is "realigned" in the next successfull upload.

What can we do to solve the problem?
I don't want to loose your good work. Thanks.

[pfeerick]

This is what springs to mind:

• if VERSION is currently non-existent, pre does not increment this time around, starts at 0.1.0
• if VERSION is > 0.1.0, i.e. 0.4.0, pre increments, ONLY if post has deleted a file to indicate upload success. meaning the file only exists between pre and post, unless there is a failure.

This then means that you can build as many times as you like, but it'll only be a successful upload that allows the patch version to increment. Would that do the trick? It would also mean all the work is done in pre, rather in post, making it less repetitive.

Hm... sounds like a state machine ...

IF NO VERSION
  START AT 0.1.0
ELSE IF VERSION BUT UPLOAD-PENDING EXISTS
  DON'T INCREMENT
ELSE IF VERSION AND UPLOAD-PENDING NOT EXISTS
  INCREMENT
ENDIF

[sblantipodi]

I need to think on this. How can pre increment only if post has deleted a file?

Are we referring to two different upload?

In this case pre is not incrementing if the previous upload was wrong?
But how do you know if there is an upload pending in the pre script?

You can know it if you launch two upload in the same time, am I missing something?

Thanks.

[pfeerick]

Ignoring the edge case of the creation of the VERSION file if not present, as I think that needs the be handled slightly differently to the normal (as in that instance, I don't think it should increment, but start at 0.1.0 - but I really don't know if that makes sense, either! 🤣 🤦‍♂️), what I am thinking of is using a file as a semaphore to link the two scripts somewhat.

the pre(build) script logic would be something like:

if file UPLOAD_PENDING exist:
   skip any further processing, leave patch number alone
else file UPLOAD_PENDING not exist:
   increment patch number by one, create empty file UPLOAD_PENDING

then the build and upload happens, if it is successful, then post(upload) script runs, with just one job: to delete the UPLOAD_PENDING file. Thus allowing the next run of the pre(build) script to increment the patch number, and create the lock file once again. And if the upload fails, the file is still there, so you can build as many times as you like, and attempt to upload as many times as you like, until it is successful. Only at this point would the UPLOAD_PENDING file be removed. Thus the build number at the end of each successful upload will reflect that of the latest successful upload, rather than a future one.

This is all on the basis (and it could just be my personal preference), that a build isn't really a new build until it has been tested (i.e. uploaded). As you might compile several times in a row merely to test your code actually compiles, before actually uploading it.

If that sounds reasonable and rationale (!!), I'll look at trying to put that together that tomorrow.

[sblantipodi]

I'm sorry but I still don't understand.
With the current script, you can build your software without uploading it as many time as you want without the needs of uploading it.
Only the upload is triggering the increment currently, but not the build.

[pfeerick]

Correct. But that gave the

version and Version.h header will always have a N+1 version than what real devices are running
problem.

So I'm thinking of doing instead is to put the increment back in the build stage, but only allow it to increment if the last upload attempt was successful. Otherwise, the build stage won't increment. So you can still build as many times as you want, but it won't increment until there has been a successful upload.

What I trying to work out as well is differences in VCS usage, programming habits, etc. Do you agree with the premise that the patch number shouldn't change just because you've pressed build, or is that sufficient to warrant the increment? As if that's the case, I think I can still handle that situation, but have a failed upload prevent an auto-increment.

[sblantipodi]

I agree that the increment should only be triggered with a successful upload. Thanks for your time pferrick I appreciate it.

[sblantipodi]

I have seen another commit. Please let me know when you think that is all ready to be merged :)

[sblantipodi]

@pfeerick I don't want to force an answer for you but just to be sure, is the pull request ready to be merged?
is there somethig missing you want to commit before the merge?

[pfeerick]

As far as I can tell, it's right to go... was just letting it settle to see that it was stable. Plus other stuff ... life... has a habit of getting in the way. 😆

Basically pre does the increment IF the lock file it wrote when it incremented last is removed by post. If it doesn't increment, it still reports the version number, and also indicates it isn't incrementing. I could hide it in the .pio folder, since that is is untracked by VCS by default, but then that brings into play the problem of multi-machine editing... better for that file to be present in a commit to keep stuff in sync.

[sblantipodi]

Many thanks for the excellent contribution.
I will add your name in the release, if you agree.

[pfeerick]

No problem. And sure... I am responsible for having written some of it... people need to know which monkey with a typewriter to blame for anything that breaks! 😛 😆