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

[messages] add AUTOPILOT_VERSION #1050

Merged
merged 7 commits into from
Dec 22, 2014
Merged

[messages] add AUTOPILOT_VERSION #1050

merged 7 commits into from
Dec 22, 2014

Conversation

flixr
Copy link
Member

@flixr flixr commented Dec 19, 2014

Proposal on adding an AUTOPILOT_VERSION message, see also #1048 for previous discussion on this...

Finally merged state:

The AUTOPILOT_VERSION message contains:

  • the version encoded as integer with MAJOR * 10000 + MINOR * 100 + PATCH
  • the complete version string as returned by paparazzi_version

This message is sent once at startup of the autopilot, but can also be registered like any normal telemetry message by adding it to the telemetry airframe file.

The version string is also displayed in the GCS alert window (once per aircraft, unless it changes)...

Also the "build version" is not written to var/build_version.txt and the server also writes it to the log.

When building an aircraft it will now warn if the last build version differs from current version.
It only checks if the current version number (e.g. 5.3.2) is contained in the full build version string. So it will warn if you changed branches to a different version, but not after e.g. committing only a airframe config change.

You can also run make print_version to show the current version (and the warning if build version differs).

@flixr
Copy link
Member Author

flixr commented Dec 20, 2014

@gautierhattenberger @dewagter @esden @martinmm
Would be great to get some feedback on what should go into the AUTOPILOT_VERSION message, how to encode the version as int and if we really send the paparazzi_version as string...

@gautierhattenberger
Copy link
Member

I don't think it is worth keeping both BOOT and AUTOPILOT_VERSION. We could send only one of them with the fields currently in AUTOPILOT_VERSION. I don't mind the name.
Concerning paparazzi_version, since it can be installed from something else than git, it seems a good idea to keep it.

@flixr
Copy link
Member Author

flixr commented Dec 20, 2014

I just thought the BOOT message was still used in some places in the ground or logalizer code, but it seems that is only the case for some long unmaintained matlab logalizer code...
If it is really not needed anymore then I agree that we should simply remove it and only add the AUTOPILOT_VERSION message...
About sending the output of paparazzi_version as string: I was a bit unsure about this as the string can become a bit long and is hence more data to send... Even if you install from a release tarball you would still have the version encoded as one 32bit int, so e.g. 50302 for v5.3.2_testing.
But I guess since we don't send that very often it is fine to send the whole string as well as that is often the most convenient one to have...

@gautierhattenberger
Copy link
Member

Actually, why do we send the version and SHA since it is already in the string version (and even more)

@flixr
Copy link
Member Author

flixr commented Dec 20, 2014

I added the integer version because that can be easily parsed.... would vote to keep that.
The git SHA1 in the version string from paparazzi_version is rather short and hence sometimes not sufficient to find the correct version.
But I guess latest reachable tag at the beginning of the version string should make it clear again, so we can probably drop that again if we send the whole string.

@flixr
Copy link
Member Author

flixr commented Dec 21, 2014

Regarding the format modifiers: is it maybe because the value is Int in all cases instead of int32 for %lu and %ld (and int64 for %Lu and %Ld)
See http://caml.inria.fr/pub/docs/manual-ocaml/libref/Printf.html

But as long as it works with %u for large 64bit values, it is fine with me...

@flixr
Copy link
Member Author

flixr commented Dec 21, 2014

The formatted_string_of_value is still failing...

@flixr
Copy link
Member Author

flixr commented Dec 21, 2014

@gautierhattenberger @martinmm @dewagter do you see any reason to keep the BOOT message?
If not I would propose to replace it with the AUTOPILOT_VERSION message with numerical and full paparazzi_version string (but without a separate git sha1 field), see master...replace_boot_with_autopilot_version

@dewagter
Copy link
Member

Didn't it contain the md5 hash of the used conf?

-Christophe

On Sun, Dec 21, 2014 at 4:59 PM, Felix Ruess notifications@github.com
wrote:

@gautierhattenberger https://github.com/gautierhattenberger @martinmm
https://github.com/martinmm @dewagter https://github.com/dewagter do
you see any reason to keep the BOOT message?
If not I would propose to replace it with the AUTOPILOT_VERSION message
with numerical and full paparazzi_version string (but without a separate
git sha1 field), see master...replace_boot_with_autopilot_version
master...replace_boot_with_autopilot_version


Reply to this email directly or view it on GitHub
#1050 (comment).

@dewagter
Copy link
Member

ow sorry, that is the ALIVE message.

As long as we keep the MD5, changing boot is fine with me.

-Christophe

On Sun, Dec 21, 2014 at 7:41 PM, Christophe De Wagter dewagter@gmail.com
wrote:

Didn't it contain the md5 hash of the used conf?

-Christophe

On Sun, Dec 21, 2014 at 4:59 PM, Felix Ruess notifications@github.com
wrote:

@gautierhattenberger https://github.com/gautierhattenberger @martinmm
https://github.com/martinmm @dewagter https://github.com/dewagter do
you see any reason to keep the BOOT message?
If not I would propose to replace it with the AUTOPILOT_VERSION message
with numerical and full paparazzi_version string (but without a separate
git sha1 field), see master...replace_boot_with_autopilot_version
master...replace_boot_with_autopilot_version


Reply to this email directly or view it on GitHub
#1050 (comment).

flixr and others added 2 commits December 22, 2014 12:12
- always save build version when updating anything that depends on libpprz, so it will also be updated on e.g. make tmtc
- only warn if the version number (major.minor.patch) is different, disregard the label, sha1, dirty postfixes in the string...
flixr added a commit that referenced this pull request Dec 22, 2014
[messages] replace BOOT with AUTOPILOT_VERSION

The AUTOPILOT_VERSION message contains:
- the version encoded as integer with `MAJOR * 10000 + MINOR * 100 + PATCH`
- the complete version string as returned by paparazzi_version

This message is sent once at startup of the autopilot, but can also be registered like any normal telemetry message by adding it to the telemetry airframe file.

The version string is also displayed in the GCS alert window (once per aircraft, unless it changes)...

Also the "build version" is not written to `var/build_version.txt` and the server also writes it to the log.

When building an aircraft it will now warn if the last build version differs from current version.
It only checks if the current version number (e.g. 5.3.2) is contained in the full build version string. So it will warn if you changed branches to a different version, but not after e.g. committing only a airframe config change.

You can also run `make print_version` to show the current version (and the warning if build version differs).
@flixr flixr merged commit bdd8d00 into master Dec 22, 2014
@flixr flixr deleted the autopilot_version_msg branch December 22, 2014 14:38
@flixr flixr added this to the v5.4 milestone Dec 22, 2014
@alonsoac
Copy link
Contributor

How often is this message sent?

On Mon, Dec 22, 2014 at 8:38 AM, Felix Ruess notifications@github.com
wrote:

Merged #1050 #1050.


Reply to this email directly or view it on GitHub
#1050 (comment).

@flixr
Copy link
Member Author

flixr commented Dec 24, 2014

It is sent once at boot. With 0f0f118 I also added it to some default telemetry modes (sent every 11.1 seconds)...

@alonsoac
Copy link
Contributor

Great. If it was just at boot it wouldn't be as useful for me
On Dec 24, 2014 3:52 AM, "Felix Ruess" notifications@github.com wrote:

It is sent once at boot. With 0f0f118
0f0f118
I also added it to some default telemetry modes (sent every 11.1 seconds)...


Reply to this email directly or view it on GitHub
#1050 (comment).

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

Successfully merging this pull request may close these issues.

4 participants