Skip to content

Add PR number to version string#1887

Merged
pfeerick merged 5 commits intomainfrom
pr_number
Apr 25, 2022
Merged

Add PR number to version string#1887
pfeerick merged 5 commits intomainfrom
pr_number

Conversation

@rotorman
Copy link
Member

@rotorman rotorman commented Apr 24, 2022

Augments #1850 by adding a pull request number to the version string.

@pfeerick
Copy link
Member

pfeerick commented Apr 24, 2022

Oh, very nice... from the build log that looks like attempt 2 did it ;) Then should be able to do something like v2.8.0-PR1887 or something instead of merge :)

btw, for the tags, am I right in thinking that for 2.8.0, since the tag for say RC1 will be v2.8.0-rc1, it is going to say v2.8.0-v2.8.0-rc1? Maybe it replace the version tag when it's a tagged version?

image

Also, I just realised another side effect of this... altering the suffix also propagates through to semver flag in radio.yml... so do we actually want to alter the version number, or add another tag (ie. in stamp.h.in) for the suffix, and use that when present in the UI? i.e. VERSION_SUFFIX would get used when present for PRs, branches, self-builds, VERSION_TAG would get used when a tagged build is detected... something like that.

@rotorman
Copy link
Member Author

Oh, very nice... from the build log that looks like attempt 2 did it ;) Then should be able to do something like v2.8.0-PR1887 or something instead of merge :)

Unfortunately it does not work (yet) - still only -merge suffix:

grafik

btw, for the tags, am I right in thinking that for 2.8.0, since the tag for say RC1 will be v2.8.0-rc1, it is going to say v2.8.0-v2.8.0-rc1? Maybe it replace the version tag when it's a tagged version?

Also, I just realised another side effect of this... altering the suffix also propagates through to semver flag in radio.yml... so do we actually want to alter the version number, or add another tag (ie. in stamp.h.in) for the suffix, and use that when present in the UI? i.e. VERSION_SUFFIX would get used when present for PRs, branches, self-builds, VERSION_TAG would get used when a tagged build is detected... something like that.

Indeed, I will look into this, as this is a good idea to split the suffix into own variable.

@rotorman rotorman force-pushed the pr_number branch 2 times, most recently from 7205ea7 to a2f42b8 Compare April 24, 2022 14:45
Prioritize EDGETX_VERSION_TAG, if set, else resort to EDGETX_VERSION_SUFFIX, if set.
Adjust also firmware_version and boot_version strings and adjist bootloader title.
@rotorman
Copy link
Member Author

rotorman commented Apr 24, 2022

I hope to have addressed all issues.

This is the result, when flashed on physical TX16S mkII with the GitHub build from this PR:

grafik

The same sourcetree, built locally and flashed on TX16S mkII:

grafik

Bootloader flashed with the binary built by this PR in GitHub:

grafik

Bootloader display, when selecting the firmware binary from this PR:

grafik

Bootloader display, when selecting the self-built binary:

grafik

The beginning of radio.yml (semver uses pure version number only):

semver: 2.8.0
board: tx16s

I have not yet tested nightly (I guess possible only after this PR is merged) or the tagged builds (could do a test tag and then delete it later).

@rotorman
Copy link
Member Author

rotorman commented Apr 24, 2022

The output on RadioMaster Zorro with this PR build:

grafik

the same codebase, but locally built:

grafik

Bootloader flashed from this PR:

grafik

Selecting firmware limited to version string only (w/o suffix or tag) due to small screen size:

grafik

@Eldenroot
Copy link
Contributor

the whole text should be visible - maybe add a posibility to select it and click on it to see all the info?

@rotorman
Copy link
Member Author

rotorman commented Apr 24, 2022

Displaying text longer than the display width could be solved with a slowly scrolling text, but this is an idea for a further PR.

@Eldenroot
Copy link
Contributor

Scrolling would be nice... and I think it should be bundled into one PR, why to create another one? Now it is only a partially done.

Omit GIT SHA from version string if a tagged version.
Otherwise, show on own line since is too long for 128x64 B&W displays.
@pfeerick
Copy link
Member

pfeerick commented Apr 25, 2022

I don't know if any changes to the bootloader are warranted... long lines have always been a problem there and enough info is conveyed for it without increasing the complexity of its code.

For the long version string (which mainly only affects 128x64 B&W screens since they don't have enough width), I've moved the commit SHA to it's own line and it's only shown if it is a non-tagged version (since if it's tagged, it's commit hash is tied to its tag).

image

Copy link
Member

@pfeerick pfeerick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic work Risto :) We can certainly push a tag to test it - although I technically the nightly build should trigger that is it is supposed to be a moving tag if I remember the github build process properly. Which now that of I think of that, it makes me think I need to re-add the git hash for tagged B&W builds... I guess we'll find out soon! 🤦 Or, actually no, as the build date is there... may not be such an issue.

edit: looks like the nightly tag might be applied as part of the release action of the nightly build, so a nightly build will come up like the above screenshot. Either that, or the tag is ignored. Will do a test tag for that later in my own repo to see what happens.

@pfeerick pfeerick merged commit ddad99e into main Apr 25, 2022
@pfeerick pfeerick deleted the pr_number branch April 25, 2022 01:42
@pfeerick pfeerick linked an issue Apr 25, 2022 that may be closed by this pull request
rotorman added a commit that referenced this pull request Apr 27, 2022
* Add PR number to version string

* Testing another way

* Decide according to GitHub ref type how to parse and where to output.
Prioritize EDGETX_VERSION_TAG, if set, else resort to EDGETX_VERSION_SUFFIX, if set.
Adjust also firmware_version and boot_version strings and adjist bootloader title.

* Consider also B/W.

* B&W: omit GIT SHA if tagged, otherwise own line
Omit GIT SHA from version string if a tagged version.
Otherwise, show on own line since is too long for 128x64 B&W displays.

Co-authored-by: Peter Feerick <peter.feerick@gmail.com>
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.

Add "*-dev" to version string of non-releases

3 participants