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

version: Expose PX4 git tag string #11676

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions src/lib/version/version.c
Original file line number Diff line number Diff line change
Expand Up @@ -347,8 +347,8 @@ uint64_t px4_firmware_version_binary(void)

const char *px4_ecl_lib_version_string(void)
{
#ifdef ECL_LIB_GIT_VERSION_STRING
return ECL_LIB_GIT_VERSION_STRING;
#ifdef ECL_LIB_GIT_VERSION_STR
return ECL_LIB_GIT_VERSION_STR;
#else
return NULL;
#endif
Expand All @@ -370,3 +370,11 @@ uint64_t px4_os_version_binary(void)
#endif
}

const char *px4_firmware_git_tag_string(void)
{
#ifdef PX4_GIT_TAG_STR
return PX4_GIT_TAG_STR;
#else
return NULL;
#endif
}
17 changes: 12 additions & 5 deletions src/lib/version/version.h
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ __EXPORT const char *px4_toolchain_name(void);
__EXPORT const char *px4_toolchain_version(void);

/**
* Firmware version as human readable string (git tag)
* Firmware version as a commit hash (git hash)
*/
__EXPORT const char *px4_firmware_version_string(void);

Expand All @@ -166,25 +166,32 @@ __EXPORT const char *px4_firmware_version_string(void);
__EXPORT const char *px4_firmware_git_branch(void);

/**
* Firmware version in binary form (first part of the git tag)
* Firmware version in binary form (first part of the git hash)
*/
__EXPORT uint64_t px4_firmware_version_binary(void);

/**
* ECL lib version as human readable string (git tag)
* ECL lib version as human readable string (git hash)
*/
__EXPORT const char *px4_ecl_lib_version_string(void);

/**
* MAVLink lib version in binary form (first part of the git tag)
* MAVLink lib version in binary form (first part of the git hash)
*/
__EXPORT uint64_t px4_mavlink_lib_version_binary(void);

/**
* Operating system version in binary form (first part of the git tag)
* Operating system version in binary form (first part of the git hash)
* @return this is not available on all OSes and can return 0
*/
__EXPORT uint64_t px4_os_version_binary(void);

/**
* Firmware version as human readable string (git tag)
* Will point to a parent tag (as returned by `git describe`) if current commit is not tagged
* @return Tag name as a C string or NULL if not defined
*/
__EXPORT const char *px4_firmware_git_tag_string(void);

__END_DECLS

6 changes: 6 additions & 0 deletions src/modules/logger/logger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2138,6 +2138,8 @@ void Logger::write_version(LogType type)

const char *git_branch = px4_firmware_git_branch();

const char *git_tag = px4_firmware_git_tag_string();

if (git_branch && git_branch[0]) {
write_info(type, "ver_sw_branch", git_branch);
}
Expand All @@ -2146,6 +2148,10 @@ void Logger::write_version(LogType type)
write_info(type, "sys_os_ver", os_version);
}

if (git_tag && git_tag[0]) {
write_info(type, "ver_sw_tag", git_tag);
Copy link
Member

Choose a reason for hiding this comment

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

I don't see why we need the tag, as we already have it through px4_firmware_version(), https://github.com/PX4/Firmware/pull/11676/files#diff-64b21add574f1f3f059c2abc18eae7d6R2121.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I understand, the px4_firmware_version() does not, in fact, return a tag, but rather a version number. That's fine for mainline firmwares, but we have modified firmware builds with additional information in the tag (our tags look like v1.8.2-clever.5). We'd like to see the whole tag to not guess which exact firmware we're using.

Copy link
Member

Choose a reason for hiding this comment

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

We have custom version tags for this: https://dev.px4.io/en/setup/building_px4.html#firmware_version. Does that work for you?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's an interesting feature, but unfortunately I don't think it does. We use different firmwares for different models of drones, and it'd be more convenient to just use the full tag name (which contains the target drone name) instead of trying to differentiate by numbers.

Copy link
Member

Choose a reason for hiding this comment

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

Having different firmwares for different models does not sound great either. Why not distinguish by using different SYS_AUTOSTART id's?

Copy link
Member

Choose a reason for hiding this comment

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

The branch name is already in the log, did you see that?

Copy link
Contributor

@dvornikov-aa dvornikov-aa Jul 9, 2019

Choose a reason for hiding this comment

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

We usually look through the log after the flight. Git tag string (and the branch name inside) is important before the flight, e.g. during the manual vehicle inspection.

Copy link
Member

Choose a reason for hiding this comment

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

That does not give me a reason why the branch is not enough.
What I want to avoid here is to have redundant information in different formats in the log. That is very confusing for anyone looking at the data.

Copy link
Contributor

Choose a reason for hiding this comment

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

The possibility of getting tag and branch name BEFORE flight as a pre-flight check would be very useful because we often have to test different features (e. g. precision landing and different camera modules) on one prototype at a close periods of time.

Copy link
Member

Choose a reason for hiding this comment

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

The possibility of getting tag and branch name BEFORE flight as a pre-flight check would be very useful because we often have to test different features (e. g. precision landing and different camera modules) on one prototype at a close periods of time.

Which you do with ver all.

}

write_info(type, "sys_os_ver_release", px4_os_version());
write_info(type, "sys_toolchain", px4_toolchain_name());
write_info(type, "sys_toolchain_ver", px4_toolchain_version());
Expand Down
13 changes: 13 additions & 0 deletions src/systemcmds/ver/ver.c
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,19 @@ int ver_main(int argc, char *argv[])
if (git_branch && git_branch[0]) {
printf("FW git-branch: %s\n", git_branch);
}

const char *git_tag = px4_firmware_git_tag_string();

if (git_tag && git_tag[0]) {
printf("FW git tag: %s\n", git_tag);
}

const char *ecl_git_hash = px4_ecl_lib_version_string();

if (ecl_git_hash && ecl_git_hash[0]) {
printf("ECL git-hash: %s\n", ecl_git_hash);
}

}

fwver = px4_os_version();
Expand Down