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

add cmake exporter for CLion use #5476

Merged
merged 10 commits into from Jan 26, 2018

Conversation

Projects
None yet
8 participants
@thinkberg
Contributor

thinkberg commented Nov 10, 2017

Description

I am working mainly with CLion and have until now written the CMakelists.txt by hand, which is very uncomfortable. This is an exporter that exports a cmake config which basically adds all the necessary defines and settings, so the IDE knows about the code and adds a custom target that will compile using the mbed-cli.

Status

IN DEVELOPMENT

I need some input on this. Everything works nicely, but for some reason the Makefile and the mbed-cli links somehow differently than what happens with cmake. Some info on the best way to configure the flags and arguments to the linker would be nice.

run mbed export -i cmake_gcc_arm to create the CMakeLists.txt

Todos

  • add support for other toolchains than GCC_ARM
@amq

This comment has been minimized.

Contributor

amq commented Nov 11, 2017

@thinkberg for debugging and SWO you need to externally use J-Link Debugger and J-Link SWO Viewer, right?

@0xc0170 0xc0170 requested a review from theotherjimmy Nov 11, 2017

@thinkberg

This comment has been minimized.

Contributor

thinkberg commented Nov 11, 2017

@amq CLion has an in-source debugger view that uses any gdb compatible gdb server. So, yes, JLinkGDBServer will work, as will pyOCD or openocd. You have to start the server in a terminal somewhere and configure a remote debug target, but otherwise, extremly comfortable IMHO. I can post a few screenshots if that is of interest.

@amq

This comment has been minimized.

Contributor

amq commented Nov 11, 2017

@thinkberg I'd be grateful for screenshots

@thinkberg

This comment has been minimized.

Contributor

thinkberg commented Nov 11, 2017

the toolchain setup (I have added an ARM toolchain)
screen shot 2017-11-11 at 21 05 09

remote debugging setup
screen shot 2017-11-11 at 21 11 32

the target selection (via the cmake exporter)
It creates a single library target for each mbed add library and a special target for the app. We need the targets, so clion finds all the source and marks them as source, so all code inspection works.
screen shot 2017-11-11 at 21 14 50

I added a project.cmake with a target that flashes to the eval board
screen shot 2017-11-11 at 21 21 10

the view after running remote debug and halting at a breakpoint
the lower right window shows the JLinkGDBServer output: JLinkGDBServer -device nRF52 -if swd -port 3333
screen shot 2017-11-11 at 21 22 40

stopping at a more interesting point
You can select variables and see their value or inspect them in the source editor. Also it displays values of variables directly in the source at line 212.
screen shot 2017-11-11 at 21 39 01

I haven't yet contacted Jetbrains for things like running the gdb server automatically etc. I guess its possible.

Of course, full source inspection works, so you get doc popups, type inspection etc.

@theotherjimmy

This looks great!

I left some comments below about how we could improve this PR even more.

def build(project_name, log_name="build_log.txt", cleanup=True):
""" Build Make project """
# > Make -j
cmd = ["make", "-j"]

This comment has been minimized.

@theotherjimmy

theotherjimmy Nov 13, 2017

Contributor

Is this the correct command to run? I was under the impression that you would need to call cmake to use a CMakeLists.txt.

This comment has been minimized.

@thinkberg

thinkberg Nov 14, 2017

Contributor

There are parts of the code that are not obvious to me, maybe you can help me understand:

When is this build called, except for cleanup?

And, actually, you do run cmake only once and then make or ninja etc. I will need to find a way to identify what is needed (like cmake -E make, but this requires that I know where the cmake build dir is in the end. I will try to find something that works.

This comment has been minimized.

@theotherjimmy

theotherjimmy Nov 14, 2017

Contributor

The Mbed OS handbook contains documentation of the bulid method. It's pretty sparse, so feel free to ask questions, and I'll answer them and update that document.

This comment has been minimized.

@thinkberg

thinkberg Nov 15, 2017

Contributor

[WIP]

@classmethod
def is_target_supported(cls, target_name):
return True

This comment has been minimized.

@theotherjimmy

theotherjimmy Nov 13, 2017

Contributor

I know this is incorrect. You don't support every target unconditionally. Could you update this to check for support much like the makefile exporers do?

This comment has been minimized.

@thinkberg

thinkberg Nov 15, 2017

Contributor

fixed.

sources = set(self.resources.c_sources + \
self.resources.cpp_sources + \
self.resources.s_sources + \
self.resources.headers)

This comment has been minimized.

@theotherjimmy

theotherjimmy Nov 13, 2017

Contributor

I don't think that headers are sources. Maybe this variable should be renamed?

This comment has been minimized.

@thinkberg

thinkberg Nov 15, 2017

Contributor

I did explain in the code, adding headers helps IDEs detect files belonging to the project. cmake ignores them. As a purist I don't like it, but it makes life easier.

libnames = [l[:-4] for l in self.resources.lib_refs]
libs = {re.sub(r'^[.]/', '', l): sorted([f for f in sources if f.startswith(l)]) for l in libnames}
libs = {k: v for k, v in libs.items() if len(v) != 0}
srcs = sorted([f for f in sources if f not in [item for sublist in libs.values() for item in sublist]])

This comment has been minimized.

@theotherjimmy

theotherjimmy Nov 13, 2017

Contributor

This looks like it might be a check for uniqueness. set(), used above, already guarantees uniqueness, so I don't think this is needed. Further, it's very hard to follow exactly what this is doing.

This comment has been minimized.

@thinkberg

thinkberg Nov 15, 2017

Contributor

explanation in the code. python has not subtract list from list, so its done like this.

self.resources.s_sources + \
self.resources.headers)
libnames = [l[:-4] for l in self.resources.lib_refs]

This comment has been minimized.

@theotherjimmy

theotherjimmy Nov 13, 2017

Contributor

despite the fact that these are .lib files, you might want to name these variable something other than lib* as they can easily be confused with .a handling. Something like dependencies or deps might be better.

This comment has been minimized.

@thinkberg

thinkberg Nov 15, 2017

Contributor

fixed.

else:
ctx['pp'] = None
for templatefile in ['cmake/%s.tmpl' % self.TEMPLATE]:

This comment has been minimized.

@theotherjimmy

theotherjimmy Nov 13, 2017

Contributor

Why are you looping through a list size one? Could you simplify this?

This comment has been minimized.

@thinkberg

thinkberg Nov 14, 2017

Contributor

I will take a look and update the PR. Some of your comments obviously relay to my lazyness...

This comment has been minimized.

@thinkberg

thinkberg Jan 19, 2018

Contributor

fixed

@thinkberg

This comment has been minimized.

Contributor

thinkberg commented Nov 15, 2017

I am still fighting with linking. Almost there... I believe my issues, like /Users/leo/Documents/Source/ubirch/mbed/cmake-test/mbed-os/rtos/TARGET_CORTEX/mbed_boot.c:487: undefined reference to `__real_main' are related to the position of the libs in the linker command line.

However, I also have a link issue like this:

/Users/leo/Documents/Source/ubirch/mbed/cmake-test/ubirch-mbed-nrf52-storage/storage/NRF52FlashStorage.cpp:139: undefined reference to `fs_init' where a dependency library doesn't find stuff in mbed-os.

I will dig a little more...

# we flatten the list of source files from all dependencies and
# then only add file to srcs if its not in that list
# (basically srcs = allSourcefiles - flatten(depSources.values())
srcs = [f for f in allSourceFiles if f not in [item for sublist in depSources.values() for item in sublist]]

This comment has been minimized.

@theotherjimmy

theotherjimmy Nov 15, 2017

Contributor

I think this may be easier with a set. For example:

srcs = set(allSourceFiles) - sum(set(dep) for dep in depSources.values(), set())

I think you mentioned something about python lists not supported subtraction. Python sets do. Maybe this was the logic you were looking for.

This comment has been minimized.

@thinkberg

thinkberg Nov 18, 2017

Contributor

I found an even simpler and more readable way with your hint.

@0xc0170 0xc0170 added needs: work and removed needs: review labels Nov 16, 2017

@thinkberg

This comment has been minimized.

Contributor

thinkberg commented Nov 18, 2017

More progress, my project now builds.

  • For some reason the hex files after objcopy (without softdevice, nrf52) differe between mbed compile and cmake build
  • the linking appears to be fragile, after sorting the static dependencies in a way libmbed-os.a is last it worked, before there were issues not finding mbed functions
  • currently the exporter uses the profile at export time. cmake would support different profiles if there was a way to get all the different command line options for each of the profiles

I still need to work on the build() for the CI.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Dec 18, 2017

@thinkberg Please rebase properly, commits from master should not be part of this patch (visible here)

@thinkberg thinkberg force-pushed the ubirch:add-cmake-export branch from e57ffa2 to 842974e Dec 18, 2017

@thinkberg

This comment has been minimized.

Contributor

thinkberg commented Dec 18, 2017

Sorry. Also for the delay, had to fix some retina detachment first. Need to restart working on it now.

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Dec 18, 2017

@thinkberg No worries. I hope you're felling better!

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jan 12, 2018

Is this PR still in progress?

@mbed-ci

This comment has been minimized.

mbed-ci commented Jan 12, 2018

Automatic CI verification build not done, please verify manually.

@thinkberg

This comment has been minimized.

Contributor

thinkberg commented Jan 12, 2018

Yes, still on it, just other work that got inbetween...

@thinkberg thinkberg force-pushed the ubirch:add-cmake-export branch from ed36734 to 7b7c4bd Jan 12, 2018

@thinkberg

This comment has been minimized.

Contributor

thinkberg commented Jan 12, 2018

I am having linker issues. All compiles fine, but the last step fails:

/usr/local/Cellar/gcc-arm-none-eabi/20160928/bin/../lib/gcc/arm-none-eabi/5.4.1/../../../../arm-none-eabi/lib/armv7e-m/softfp/libc.a(lib_a-signalr.o): In function `_kill_r':
signalr.c:(.text._kill_r+0x10): undefined reference to `_kill'
/usr/local/Cellar/gcc-arm-none-eabi/20160928/bin/../lib/gcc/arm-none-eabi/5.4.1/../../../../arm-none-eabi/lib/armv7e-m/softfp/libc.a(lib_a-signalr.o): In function `_getpid_r':
signalr.c:(.text._getpid_r+0x0): undefined reference to `_getpid'

Sounds similar to this issue: gnu-mcu-eclipse/eclipse-plugins#58 However, I wonder how to fix it.

@thinkberg

This comment has been minimized.

Contributor

thinkberg commented Jan 12, 2018

Got it working, by adding dependencies for stdc++, c, and nosys (well, actually all system deps) to the final target. The leads to cmake adding them after the targets.

Feel free to test and review.

One issue left is: some of the libraries built will have a few too many source files if they have sub-dependencies. However, this does not seem to influence the built.

@thinkberg

This comment has been minimized.

Contributor

thinkberg commented Jan 12, 2018

still need to work on the [WIP] above. Almost forgot about it.

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Jan 12, 2018

@thinkberg Thanks for getting to this! It looks like it's coming along quite well.

@thinkberg

This comment has been minimized.

Contributor

thinkberg commented Jan 13, 2018

After finding the command line to run the examples test:

python ../tools/test/examples/examples.py export cmake_gcc_arm

I added the required steps to build. It works for me with the blinky example.

@thinkberg

This comment has been minimized.

Contributor

thinkberg commented Jan 13, 2018

@theotherjimmy: I get errors like this when doing the test builds:

/usr/local/Cellar/gcc-arm-none-eabi/20160928/bin/../lib/gcc/arm-none-eabi/5.4.1/../../../../arm-none-eabi/bin/ld: cannot find -lublox-odin-w2-driver

Is this expected?

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Jan 14, 2018

I get errors like this when doing the test builds:

/usr/local/Cellar/gcc-arm-none-eabi/20160928/bin/../lib/gcc/arm-none-eabi/5.4.1/../../../../arm-none-eabi/bin/ld: cannot find -lublox-odin-w2-driver

Is this expected?

No build failures are expect.

It looks like you may need to add resources.lib_dirs knowlage to your exporter. For reference the makefile template add these resources to the jinja context.

@thinkberg

This comment has been minimized.

Contributor

thinkberg commented Jan 15, 2018

added the resources libs feature, the library is now found

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Jan 22, 2018

@thinkberg Yep. You got it!

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Jan 22, 2018

I'm going to lunch. I'll review this PR when I get back.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jan 23, 2018

I'm going to lunch. I'll review this PR when I get back.

And has not returned yet 🍴😆

@thinkberg

This comment has been minimized.

Contributor

thinkberg commented Jan 23, 2018

Lunch coma. Must have been awesome food.

# optional custom target to build via mbed-cli
ADD_CUSTOM_TARGET(mbed-cli-build
COMMAND ${CMAKE_COMMAND} -E echo "mbed compile --build BUILD/${CMAKE_BUILD_TYPE} --profile ${MBED_BUILD_PROFILE}"
COMMAND mbed compile --build BUILD/${CMAKE_BUILD_TYPE} --profile ${MBED_BUILD_PROFILE}

This comment has been minimized.

@theotherjimmy

theotherjimmy Jan 23, 2018

Contributor

Is this the primary compiling method?

This comment has been minimized.

@thinkberg

thinkberg Jan 23, 2018

Contributor

No, that’s just for convenience. The primary method is to choose the {{name}} target.

This comment has been minimized.

@theotherjimmy

theotherjimmy Jan 23, 2018

Contributor

Thanks.

@cmonr cmonr added needs: CI and removed needs: review labels Jan 23, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jan 23, 2018

/morph build

1 similar comment
@cmonr

This comment has been minimized.

Contributor

cmonr commented Jan 23, 2018

/morph build

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jan 23, 2018

@thinkberg, is it fair to say that this PR will not be getting export functionality for other toolchains?

@thinkberg

This comment has been minimized.

Contributor

thinkberg commented Jan 23, 2018

Not this one. However, it should be easy to do one has access to armcc etc. I just didn’t have the time.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jan 23, 2018

That's fine. Just wanted to make sure.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jan 25, 2018

/morph build

1 similar comment
@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jan 26, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Jan 26, 2018

Build : SUCCESS

Build number : 973
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5476/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@cmonr cmonr merged commit 6e964a6 into ARMmbed:master Jan 26, 2018

19 checks passed

ARM mbed CI Verification build successful.
Details
AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Local events testing has passed
Details
travis-ci/littlefs Local littlefs testing has passed
Details
travis-ci/mbed2-ATMEL Local mbed2-ATMEL testing has passed
Details
travis-ci/mbed2-MAXIM Local mbed2-MAXIM testing has passed
Details
travis-ci/mbed2-NORDIC Local mbed2-NORDIC testing has passed
Details
travis-ci/mbed2-NUVOTON Local mbed2-NUVOTON testing has passed
Details
travis-ci/mbed2-NXP Local mbed2-NXP testing has passed
Details
travis-ci/mbed2-RENESAS Local mbed2-RENESAS testing has passed
Details
travis-ci/mbed2-SILICON_LABS Local mbed2-SILICON_LABS testing has passed
Details
travis-ci/mbed2-STM Local mbed2-STM testing has passed
Details
travis-ci/tools Local tools testing has passed
Details

@cmonr cmonr removed the ready for merge label Jan 26, 2018

@drewcassidy

This comment has been minimized.

Contributor

drewcassidy commented Jan 31, 2018

I know this has been closed and merged but I'm having a few issues with it

  • it generates a getting started html file every time you update the cmake file
  • the cmake-build-debug directory seems to confuse the compiler. I was having errors where it couldn't find BLE.h because it was searching in the cmake build directory instead. Maybe this should automatically add cmake-build-debug to .mbedignore?
@cmonr

This comment has been minimized.

Contributor

cmonr commented Jan 31, 2018

@drewcassidy I would suggest opening an issue, and linking it to this PR. We generally stop paying attention to pull request comments after they've been merged.

@thinkberg

This comment has been minimized.

Contributor

thinkberg commented Jan 31, 2018

Please open an issue and I will take a look. The GettingStarted.html is generated by the exporter every time. Not sure how to stop that. And a hint as how to add a line safely to .mbedignore would be nice.

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Jan 31, 2018

And a hint as how to add a line safely to .mbedignore would be nice.

You could also generate a .mbedignore file containing:

*

within cmake-build-debug, avoiding the whole safety question.

@elmot

This comment has been minimized.

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