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

C++ Support doesn't work #5695

Closed
zigarrre opened this issue Jan 16, 2018 · 7 comments · Fixed by #21397
Closed

C++ Support doesn't work #5695

zigarrre opened this issue Jan 16, 2018 · 7 comments · Fixed by #21397
Assignees
Labels
area: C++ bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug
Milestone

Comments

@zigarrre
Copy link

zigarrre commented Jan 16, 2018

Problem

When trying to use Zephyr with C++ I noticed that a lot of the headers contain code that isn't valid in C++ and causes compile errors when included in C++ files. I noticed this mainly with the Bluetooth subsystem but I wouldn't be surprised if other parts of are affected too as I haven't tested them.

So even though my examples focus on Bluetooth this issue concerns Zephyr as a whole.

The documentation claims that Zephyr has C++ support, but at the moment not even the most basic things work.

After asking on IRC about this @SebastianBoe confirmed those problems and submitted PR #5642 with some small fixes. While that was a first step in the right direction a lot more has to be done still.

As far as I know there are absolutely no automated tests in Zephyr for C++ compatibility at the moment. I would suggest changing that to properly ensure C++ compatibility in the future. If possible maybe just reuse/adopt existing C tests and additionally compile them in C++.

Solution

  1. Fix any headers that contain invalid C++ code to be valid in both C and C++.
  2. Add automated tests for C++ compatibility

Minimal Example

#include <zephyr.h>

#include <bluetooth/bluetooth.h>
#include <bluetooth/hci.h>
#include <bluetooth/conn.h>
#include <bluetooth/uuid.h>
#include <bluetooth/gatt.h>

static const struct bt_data ad[] = {
    BT_DATA_BYTES(BT_DATA_FLAGS, (BT_LE_AD_GENERAL | BT_LE_AD_NO_BREDR))
};

void on_bt_ready(int err) {
    bl_le_adv_start(BT_LE_ADV_CONN, ad, ARRAY_SIZE(ad), NULL, NULL);
}

int main() {
    bt_enable(on_bt_ready);
}

The following configuration was used in prj.conf:

CONFIG_CPLUSPLUS=y
CONFIG_BT=y
CONFIG_BT_DEBUG_LOG=y
CONFIG_BT_DEVICE_NAME="Test beacon"

Built with the current master branch of zephyr and zephyr-sdk-0.9.2 for the nrf52 platform using the following commands:

mkdir build && cd build
cmake -DBOARD=nrf52_pca10040 ..
make

This minimal example will generate the following errors:

In file included from /home/zigarrre/coding/zephyr_test/src/test.cpp:3:0:
/home/zigarrre/coding/zephyr_test/src/test.cpp: In function ‘void on_bt_ready(int)’:
/home/zigarrre/coding/z/zephyr/include/bluetooth/bluetooth.h:149:4: error: taking address of temporary [-fpermissive]
    })
    ^
/home/zigarrre/coding/z/zephyr/include/bluetooth/bluetooth.h:151:24: note: in expansion of macro ‘BT_LE_ADV_PARAM’
 #define BT_LE_ADV_CONN BT_LE_ADV_PARAM(BT_LE_ADV_OPT_CONNECTABLE, \
                        ^~~~~~~~~~~~~~~
/home/zigarrre/coding/zephyr_test/src/test.cpp:14:21: note: in expansion of macro ‘BT_LE_ADV_CONN’
     bl_le_adv_start(BT_LE_ADV_CONN, ad, ARRAY_SIZE(ad), NULL, NULL);
                     ^~~~~~~~~~~~~~
In file included from /home/zigarrre/coding/z/zephyr/include/kernel.h:27:0,
                 from /home/zigarrre/coding/z/zephyr/include/zephyr.h:18,
                 from /home/zigarrre/coding/zephyr_test/src/test.cpp:1:
/home/zigarrre/coding/z/zephyr/include/misc/util.h:42:35: error: ‘__builtin_types_compatible_p’ was not declared in this scope
            __typeof__(&(array)[0])))
                                   ^
/home/zigarrre/coding/z/zephyr/include/misc/util.h:34:66: note: in definition of macro ‘ZERO_OR_COMPILE_ERROR’
 #define ZERO_OR_COMPILE_ERROR(cond) ((int) sizeof(char[1 - 2 * !(cond)]) - 1)
                                                                  ^~~~
/home/zigarrre/coding/z/zephyr/include/misc/util.h:48:20: note: in expansion of macro ‘IS_ARRAY’
  ((unsigned long) (IS_ARRAY(array) + \
                    ^~~~~~~~
/home/zigarrre/coding/zephyr_test/src/test.cpp:14:41: note: in expansion of macro ‘ARRAY_SIZE’
     bl_le_adv_start(BT_LE_ADV_CONN, ad, ARRAY_SIZE(ad), NULL, NULL);
                                         ^~~~~~~~~~
/home/zigarrre/coding/z/zephyr/include/misc/util.h:34:51: error: expected primary-expression before ‘char’
 #define ZERO_OR_COMPILE_ERROR(cond) ((int) sizeof(char[1 - 2 * !(cond)]) - 1)
                                                   ^
/home/zigarrre/coding/z/zephyr/include/misc/util.h:40:2: note: in expansion of macro ‘ZERO_OR_COMPILE_ERROR’
  ZERO_OR_COMPILE_ERROR( \
  ^~~~~~~~~~~~~~~~~~~~~
/home/zigarrre/coding/z/zephyr/include/misc/util.h:48:20: note: in expansion of macro ‘IS_ARRAY’
  ((unsigned long) (IS_ARRAY(array) + \
                    ^~~~~~~~
/home/zigarrre/coding/zephyr_test/src/test.cpp:14:41: note: in expansion of macro ‘ARRAY_SIZE’
     bl_le_adv_start(BT_LE_ADV_CONN, ad, ARRAY_SIZE(ad), NULL, NULL);
                                         ^~~~~~~~~~
/home/zigarrre/coding/z/zephyr/include/misc/util.h:34:51: error: expected ‘)’ before ‘char’
 #define ZERO_OR_COMPILE_ERROR(cond) ((int) sizeof(char[1 - 2 * !(cond)]) - 1)
                                                   ^
/home/zigarrre/coding/z/zephyr/include/misc/util.h:40:2: note: in expansion of macro ‘ZERO_OR_COMPILE_ERROR’
  ZERO_OR_COMPILE_ERROR( \
  ^~~~~~~~~~~~~~~~~~~~~
/home/zigarrre/coding/z/zephyr/include/misc/util.h:48:20: note: in expansion of macro ‘IS_ARRAY’
  ((unsigned long) (IS_ARRAY(array) + \
                    ^~~~~~~~
/home/zigarrre/coding/zephyr_test/src/test.cpp:14:41: note: in expansion of macro ‘ARRAY_SIZE’
     bl_le_adv_start(BT_LE_ADV_CONN, ad, ARRAY_SIZE(ad), NULL, NULL);
                                         ^~~~~~~~~~
/home/zigarrre/coding/zephyr_test/src/test.cpp:14:68: error: expected ‘)’ before ‘;’ token
     bl_le_adv_start(BT_LE_ADV_CONN, ad, ARRAY_SIZE(ad), NULL, NULL);
                                                                    ^
/home/zigarrre/coding/zephyr_test/src/test.cpp:14:68: error: expected ‘)’ before ‘;’ token
/home/zigarrre/coding/zephyr_test/src/test.cpp:14:68: error: expected ‘)’ before ‘;’ token
/home/zigarrre/coding/zephyr_test/src/test.cpp: In function ‘int main()’:
/home/zigarrre/coding/zephyr_test/src/test.cpp:19:1: warning: no return statement in function returning non-void [-Wreturn-type]
 }
 ^
make[2]: *** [CMakeFiles/app.dir/build.make:303: CMakeFiles/app.dir/src/test.cpp.obj] Error 1
make[1]: *** [CMakeFiles/Makefile2:228: CMakeFiles/app.dir/all] Error 2
make: *** [Makefile:84: all] Error 2

Full Examples

For additional, more complete code samples just use the various (Bluetooth) examples that Zephyr already provides and compile them as C++ by changing the extensions of the files as well as in the CMakeFiles.txt to .cpp.

I would suggest starting with bluetooth/beacon which is the most simple example and working our way up from there to more complex examples.

@pfalcon
Copy link
Contributor

pfalcon commented Jan 16, 2018

Based on the title, this is duplicate of #1205, #4028.

@SebastianBoe
Copy link
Collaborator

Kind of, I would suggest that when this is resolved one should look into if the fix has affected #1205 or #4028.

#4028 looks like a feature-request for supporting more C++ features.

#1205 looks like a report that C++ is broken when -fpermissive is not applied (I have already tested that neither -fpermissive or -fno-permissive resolves the issues reported here).

@nashif nashif added the Enhancement Changes/Updates/Additions to existing features label Jan 30, 2018
@jhedberg
Copy link
Member

jhedberg commented Feb 9, 2018

Note that you don't need to use the BT_MESH_ADV_CONN macro if you don't want to. Creating a "normal" struct bt_mesh_adv_param stack variable and passing a reference to that to bt_mesh_adv_start() should make a C++ compiler happy.

@DC11011100
Copy link

DC11011100 commented Apr 5, 2019

As I was trying to port my freeRTOS C++ applications, I'm sad this has stagnated and that I won't be able to use the full functionality/abstraction zephyr has to offer.

Given issues like this still exist, I think it's only fair to ATLEAST put a caveat in the documentation, which seems to sell zephyr as conveniently c++ compatible (it's not at the moment), or just remove the claim completely.

@andrewboie andrewboie added bug The issue is a bug, or the PR is fixing a bug and removed Enhancement Changes/Updates/Additions to existing features labels Apr 6, 2019
@andrewboie andrewboie added this to the v1.15.0 milestone Apr 6, 2019
@andrewboie andrewboie changed the title C++ Support C++ Support doesn't work Apr 6, 2019
@nashif nashif added the priority: medium Medium impact/importance bug label Apr 6, 2019
@nashif nashif modified the milestones: v1.15.0, future Apr 9, 2019
@nashif nashif self-assigned this May 21, 2019
@nashif nashif removed this from the future milestone May 21, 2019
@pabigot pabigot self-assigned this Aug 19, 2019
@pabigot
Copy link
Collaborator

pabigot commented Aug 19, 2019

Much of this has been addressed in the past 18 months. As soon as #18243, #18245, and #18242 are merged Zephyr should be pretty compatible with C++.

@nashif nashif removed their assignment Aug 20, 2019
@jenmwms jenmwms added this to the v2.1.0 milestone Aug 23, 2019
@pabigot
Copy link
Collaborator

pabigot commented Oct 8, 2019

Per above comment #18243, #18245, and #18242 are closed so we'll call this done.

@pabigot pabigot closed this as completed Oct 8, 2019
@pabigot
Copy link
Collaborator

pabigot commented Oct 8, 2019

Nope, sorry. This references Bluetooth specifically so #18551 blocks it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: C++ bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants