Skip to content

[nrf-ble-driver] Initial version of nrf-ble-driver#5938

Merged
grdowns merged 14 commits intomicrosoft:masterfrom
kenr:nrf-ble-driver
May 21, 2019
Merged

[nrf-ble-driver] Initial version of nrf-ble-driver#5938
grdowns merged 14 commits intomicrosoft:masterfrom
kenr:nrf-ble-driver

Conversation

@kenr
Copy link
Contributor

@kenr kenr commented Apr 3, 2019

This pull-request adds nrf-ble-driver.

nrf-ble-driver consists of a set of static and shared libraries that provide Bluetooth Low Energy connectivity
through the use of a PC and a Nordic Semiconductor System on Chip. The API on the PC side is almost identical to the SoftDevice API . SoftDevice is a Bluetooth Low Energy wireless protocol stack library.

@msftclas
Copy link

msftclas commented Apr 3, 2019

CLA assistant check
All CLA requirements met.

@Rastaban Rastaban changed the title Initial version of nrf-ble-driver [nrf-ble-driver] Initial version of nrf-ble-driver Apr 4, 2019
@kenr
Copy link
Contributor Author

kenr commented Apr 4, 2019

I'm not sure what Vcpkg-PR-Eager does.
Is there something I should do to fix this?

@Rastaban
Copy link
Contributor

Rastaban commented Apr 4, 2019

Vcpkg-PR-Eager tried to install the port on multiple triplets and fails if there are any new build failures on any triplet. Because this is a new port some of those failures may be by design if the port does not support it. for nrf-ble-driver I see a passing build on x64-osx but failing on all others. Is this expected? Let me know if you need me to attach any of the logs from the test to this PR.

@kenr
Copy link
Contributor Author

kenr commented Apr 4, 2019

I've had the port build on Windows (x64,x86) and Linux (x64). I tried it now again and it worked (see output below).
I think the logs from your build setup would help me to better understand what is going on.

Windows:
vcpkg install nrf-ble-driver:x86-windows

Installing package nrf-ble-driver[core]:x86-windows... done
Elapsed time for package nrf-ble-driver:x86-windows: 1.528 min

Total elapsed time: 2.355 min

The package nrf-ble-driver:x86-windows provides CMake targets:

    find_package(nrf-ble-driver CONFIG REQUIRED)
    # Note: 4 target(s) were omitted.
    target_link_libraries(main PRIVATE nrf::nrf_ble_driver_sd_api_v2_shared nrf::nrf_ble_driver_sd_api_v2_static nrf::nrf_ble_driver_sd_api_v3_shared nrf::nrf_ble_driver_sd_api_v3_static)

vcpkg install nrf-ble-driver:x64-windows

Installing package nrf-ble-driver[core]:x64-windows... done
Elapsed time for package nrf-ble-driver:x64-windows: 2.385 min

Total elapsed time: 2.561 min

The package nrf-ble-driver:x64-windows provides CMake targets:

    find_package(nrf-ble-driver CONFIG REQUIRED)
    # Note: 4 target(s) were omitted.
    target_link_libraries(main PRIVATE nrf::nrf_ble_driver_sd_api_v2_shared nrf::nrf_ble_driver_sd_api_v2_static nrf::nrf_ble_driver_sd_api_v3_shared nrf::nrf_ble_driver_sd_api_v3_static)



Ubuntu 18.04 (WSL):
vcpkg install nrf-ble-driver

Installing package nrf-ble-driver[core]:x64-linux... done
Elapsed time for package nrf-ble-driver:x64-linux: 1.275 min

Total elapsed time: 1.561 min

The package nrf-ble-driver:x64-linux provides CMake targets:

    find_package(nrf-ble-driver CONFIG REQUIRED)
    # Note: 4 target(s) were omitted.
    target_link_libraries(main PRIVATE nrf::nrf_ble_driver_sd_api_v2_shared nrf::nrf_ble_driver_sd_api_v2_static nrf::nrf_ble_driver_sd_api_v3_shared nrf::nrf_ble_driver_sd_api_v3_static)

@kenr
Copy link
Contributor Author

kenr commented Apr 10, 2019

Any progress on this one, anything I can assist with?

@grdowns grdowns self-assigned this Apr 12, 2019
@grdowns
Copy link
Contributor

grdowns commented Apr 12, 2019

Hey @kenr, sorry for the slow going PR. I've been able to build nrf-ble-driver for the ports in question locally as well. It seems that our Linux machine doesn't have libudev installed, and the Windows builds are unable to find the git executable. I'll be digging into these problems soon. Otherwise, everything looks good! Takk for alt :)

@kenr
Copy link
Contributor Author

kenr commented Apr 24, 2019

Hi @grdowns, thank you for look into this . Do you know when it can be merged with master? Takk for hjelpen så langt :-)

@kenr
Copy link
Contributor Author

kenr commented May 10, 2019

@grdowns How is going with the git on Windows and libudev-dev issue on Linux?

@cbezault cbezault removed their assignment May 20, 2019
@grdowns
Copy link
Contributor

grdowns commented May 21, 2019

Hei @kenr, jeg beklager den lange ventetid! I just wanted to give you an update as this PR hasn't slipped by me. The Git not found problem is by design, as our CI machines do not have Git added to their environment PATH variable. A find_program(git) from portfile.cmake allows vcpkg's tools directory to be added to the search path, which when appended to PATH it will honor the user's git instance over vcpkg's if there exists one. The added benefit of all this is users of nrf-ble-driver via vcpkg will be able to use vcpkg's git executable if they do not have one on PATH.

Additionally, I've been working on enabling nrf-ble-driver for other triplets, notably I have x64-windows-static building as well as arm64-windows. Takk for alt igjen!

Edit: As far as the libudev-dev dependency, the current practice is to simply add a system dependency message when the build start, which will display the line used to download the dependencies from apt as it's the most prominent package manager. If users rely on a different package manage they will be able to adapt the command for their uses or get the dependency on their own. I've added a message for nrf-ble-driver in portfile.cmake

@grdowns
Copy link
Contributor

grdowns commented May 21, 2019

One more update, I was looking into enabling arm-uwp and x64-uwp but noticed that the code in question in COM-centric as opposed to WinRT, which UWP is based on. Rather than go down the rabbit hole to get it to compile and run successfully, I'll leave it untouched. The port is good to go in now -- thanks for the port contribution, @kenr!

@grdowns grdowns merged commit 75bcb1c into microsoft:master May 21, 2019
@kenr
Copy link
Contributor Author

kenr commented May 22, 2019

Takk selv :-)

Just some comments, the 001-arm64-support.patch, should it be there at all? The macOS part of the file is the only code that does anything and the linking described is taken care of in nrf-ble-driver code.

Regarding the check for darwin in portfile.cmake. Darwin does not require libudev, I'm not even sure if it exists for Darwin.

We have released a new version of nrf-ble-driver now (v4.1.1). Shall I just do a new PR to get it in? I guess it will be a bit faster now that we have made things compile on the vcpkg build servers?

@grdowns
Copy link
Contributor

grdowns commented May 31, 2019

Hi @kenr!

I've created PR #6699 to both update the port to 4.1.1 and fix the system dependencies message. For future updates, you can simply change the release tag, SHA, and CONTROL file as I've done in the PR. And you are correct, future updates are quite simple now that the CI system issues have been fixed!

As for the patch, applying it adds advapi32 to the linker list for the nrf-ble-driver-sd_api_v2 CMake target on builds targeting Windows. This fixes a linker error when the user attempts to build the port for the arm64-windows triplet. In the case that the user builds x86-windows for example, the library is not needed and therefore it will not be linked in.

Let me know if you have any more questions, and thanks again for the port contribution!

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.

5 participants