Include TM2 firmware directly #3647

radfordi · 2019-04-01T20:02:16Z

This has a few nice features:

Downloads the firmware with SHA1 hash to avoid having to ever re-download
Downloads the firmware into a versioned file to avoid having to re-download
when changing branches/versions
Avoids parsing the firmware
Avoids the process of creating huge headers with cmake (portable, but slow)
Avoids regenerating files on every cmake run
Greatly Speed up running cmake in the normal case
Simplifies the cmake code drastically
The fw/ directory should drop into any libtm replacement
Only writes into the build directory (not the source directory)

but it has a few drawbacks as well:

Less portable technique
~~We may need a few changes for Windows when switching between static and dynamic libraries~~
Doesn't support local firmware development as well as the old code
Currently missing a compile-time sanity check for the central app version

Most of the drawbacks can easily be fixed, except for the portability which should be outweighed by the other benefits. This is based on and should be included into #3507, but it might need a bit more testing and I don't want to hold #3507 up. CC @claudiofantacci.

matkatz · 2019-04-02T14:30:27Z

third-party/libtm/CMakeLists.txt

@@ -11,12 +11,12 @@ set(INFRA_INCLUDE_DIR "${LIBTM_ROOT}/libtm/src/infra")
 set(LIBTM_SRC_DIR "${LIBTM_ROOT}/libtm/src")
 set(LIBTM_RESOURCES_DIR "${LIBTM_ROOT}/resources")

-include(versions.cmake)
+set(HOST_VERSION "0.19.3.1711")


is this version maintained?

I have no idea if this version number is used outside this project, but it is used to extract LIBTM_VERSION_MAJOR, etc., so I kept it for now. I'd be happy to remove it.

matkatz · 2019-04-02T14:33:37Z

CMake/external_libusb.cmake

-
-set(USE_EXTERNAL_USB ON)
-set(LIBUSB_LOCAL_INCLUDE_PATH ${CMAKE_CURRENT_BINARY_DIR}/third-party/libusb)
+if (APPLE)


can't it be moved to apple_config?

This is really part of the exported interface of libusb, so I think it should stay in a libusb specific place. Normally this would come from a target_* function if we were actually compiling libusb, an installed cmake file, or a pkg-config file, but since we are using an "external project" without installation, we need to do it ourselves.

Also, I removed (cc40ab3) apple_config.cmake as it wasn't being used. :)

dorodnic · 2019-04-02T14:39:18Z

This worked well for me on Windows, except:
You have called ADD_LIBRARY for library fw without any source files. This typically indicates a problem with your CMakeLists.txt file warning. Any idea?
Another finding is that with the PR, when setting FORCE_WINUSB_UVC the build fails for some reason. This is driving the Win7 SDK, so this can be a problem.
In fw_target.h:16, (as well as fw_central and fw_central_app) changing ::GetModuleHandle to ::GetModuleHandleA solves the problem, and at least to me it makes sense since it is being called with non-unicode string.

claudiofantacci · 2019-04-02T14:45:25Z

Hey thanks for calling me here! I was not able to have a look into it today and I have some other things tomorrow, but I will have a look asap, possibly within next Friday 👍🏻

radfordi · 2019-04-02T15:55:07Z

Thanks for debugging that @dorodnic! I (force) pushed a fix to use A and to move one of the files to the add_library call itself (instead of doing so through target_source) to avoid the warning (which oddly I wasn't getting myself).

We have a CI build that uses Win7 and FORCE_WINUSB_UVC which passed. Was it failing for you locally?

dorodnic · 2019-04-02T16:00:52Z

Another thing I noticed is:

Configuring done
CMake Error: install(EXPORT "realsense2Targets" ...) includes target "realsense2" which requires target "usb" that is not in the export set.
CMake Error: install(EXPORT "realsense2Targets" ...) includes target "realsense2" which requires target "tm" that is not in the export set.
Generating done

When setting BUILD_SHARED_LIBS=false. Then, it builds successfully.

We have a CI build that uses Win7 and FORCE_WINUSB_UVC which passed. Was it failing for you locally?

Yes, basically I like to sanity check everything locally. Not sure why it failed only for me, but the fix seem to make sense.

radfordi · 2019-04-03T04:00:23Z

@dorodnic, I updated both #3507 and #3647 with fixes to the issues you mentioned, except for the FORCE_WINUSB_UVC=True issue which I can't reproduce.

claudiofantacci · 2019-04-03T16:24:48Z

Hi @radfordi, I tried to configure and compile this PR with ninja build, under Windows 10, with the standard options and unfortunately it does not work. I get the following error:

ninja: error: 'third-party/libtm/fw/fw.dir/fw.res', needed by 'realsense2.dll', missing and no known rule to make it

It instead works using Visual Studio 15 2017 toolchain (via CMake generator).

I'll try to have a look at the CMake code of this PR, and specifically for the libtm target, trying to help.

radfordi · 2019-04-05T21:53:53Z

Rebased on v2.20.0.

This has a few nice features: - Downloads the firmware with SHA1 hash to avoid having to ever re-download - Downloads the firmware into a versioned file to avoid having to re-download when changing branches/versions - Avoids parsing the firmware - Avoids the process of creating huge headers with cmake (portable, but **slow**) - Avoids regenerating files on every cmake run - Greatly Speed up running cmake in the normal case - Simplifies the cmake code drastically - The fw/ directory should drop into any `libtm` replacement - Only writes into the build directory (not the source directory) but it has a few drawbacks as well: - Less portable technique - Doesn't support local firmware development as well as the old code - Currently missing a compile-time sanity check for the central app version Most of the drawbacks can easily be fixed, except for the portability which should be outweighed by the other benefits.

…cit export

matkatz · 2019-04-16T12:12:00Z

Android gradle solution built successfully.

ev-mp

@radfordi , great contribution!
Really appreciated.

I redirected this PR to the development branch in order to avoid extra-merges
This makes the other 2507 PR redundant.

bfulkers-i requested a review from dorodnic April 1, 2019 20:28

bfulkers-i added the T260 series Intel® T265 library label Apr 1, 2019

radfordi force-pushed the libtm-fw-incbin branch 10 times, most recently from 8eb84bb to d610028 Compare April 2, 2019 05:35

matkatz reviewed Apr 2, 2019

View reviewed changes

dorodnic added the cmake label Apr 2, 2019

dorodnic mentioned this pull request Apr 2, 2019

CMake improvements for Windows build #3611

Merged

radfordi force-pushed the libtm-fw-incbin branch 2 times, most recently from e172425 to 78e85d7 Compare April 2, 2019 15:49

radfordi force-pushed the libtm-fw-incbin branch 6 times, most recently from b7d1118 to e94a2ed Compare April 2, 2019 23:59

radfordi added 10 commits April 5, 2019 14:28

Remove obsolete apple_config.cmake

5e09bec

Fix typo in central app version parsing

895e734

Avoid creating version files in the source directory

acf294f

Match GCC diagnostic push with pop in libuvc

9db097a

Add an install target of system libusb interface

8053c1f

Avoid trigging new builds of internal libusb

78663df

Remove pointless cmake messages from libtm

45b06bd

Avoid code page warnings for external project libusb

42cdafd

Let libtm export its own include directory

a57d214

Add parenthesis to macro arguments to avoid precedence issues

2718030

radfordi force-pushed the libtm-fw-incbin branch 2 times, most recently from ad3850d to 13f3400 Compare April 5, 2019 21:52

Use extra parens for assignments within if's to avoid warning

7ec1b01

radfordi force-pushed the libtm-fw-incbin branch from 13f3400 to 6367443 Compare April 16, 2019 03:01

radfordi and others added 7 commits April 15, 2019 20:08

Avoid non-standard fourcc C syntax

036b8f2

Append CMake/ to the module path so we can include from any directory

418f5f1

Include libusb_config on demand instead of globally

d9b8097

Include android_config in preference to windows_config or unix_config

cc00611

Resources don't get picked up from static libraries with out an expli…

585b08b

…cit export

Remove old libtm firwmare header generation

1cd948f

radfordi force-pushed the libtm-fw-incbin branch from 6367443 to 1cd948f Compare April 16, 2019 03:09

ev-mp changed the base branch from master to development April 16, 2019 13:53

ev-mp approved these changes Apr 16, 2019

View reviewed changes

ev-mp merged commit ca5ce87 into IntelRealSense:development Apr 16, 2019

radfordi deleted the libtm-fw-incbin branch April 16, 2019 16:34

radfordi mentioned this pull request Apr 30, 2019

libtm build fail #3483

Closed

ClimbsRocks mentioned this pull request May 17, 2019

Include could not find load file: libusb_config #4009

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Include TM2 firmware directly #3647

Include TM2 firmware directly #3647

radfordi commented Apr 1, 2019 •

edited

Loading

matkatz Apr 2, 2019

radfordi Apr 2, 2019

matkatz Apr 2, 2019

radfordi Apr 2, 2019

radfordi Apr 2, 2019

dorodnic commented Apr 2, 2019

claudiofantacci commented Apr 2, 2019

radfordi commented Apr 2, 2019

dorodnic commented Apr 2, 2019 •

edited

Loading

radfordi commented Apr 3, 2019

claudiofantacci commented Apr 3, 2019

radfordi commented Apr 5, 2019

matkatz commented Apr 16, 2019

ev-mp left a comment

Include TM2 firmware directly #3647

Include TM2 firmware directly #3647

Conversation

radfordi commented Apr 1, 2019 • edited Loading

matkatz Apr 2, 2019

Choose a reason for hiding this comment

radfordi Apr 2, 2019

Choose a reason for hiding this comment

matkatz Apr 2, 2019

Choose a reason for hiding this comment

radfordi Apr 2, 2019

Choose a reason for hiding this comment

radfordi Apr 2, 2019

Choose a reason for hiding this comment

dorodnic commented Apr 2, 2019

claudiofantacci commented Apr 2, 2019

radfordi commented Apr 2, 2019

dorodnic commented Apr 2, 2019 • edited Loading

radfordi commented Apr 3, 2019

claudiofantacci commented Apr 3, 2019

radfordi commented Apr 5, 2019

matkatz commented Apr 16, 2019

ev-mp left a comment

Choose a reason for hiding this comment

radfordi commented Apr 1, 2019 •

edited

Loading

dorodnic commented Apr 2, 2019 •

edited

Loading