C++ wrappers

This forum is dedicated to feedback, discussions about ongoing or future developments, ideas and suggestions regarding the ChibiOS projects are welcome. This forum is NOT for support.
Bogdan
Posts: 21
Joined: Fri Apr 08, 2016 4:33 pm
Has thanked: 2 times
Been thanked: 3 times

Re: C++ wrappers

Post by Bogdan »

Hi All,

For information.

I use classes from wrapper in my own classes. I create my classes using new(). Because memory allocated don't initialized with zero, I had some problems with used timers and other. I had unpredictable behavior of my program.

I have added in wrapper in constructors memset(&data, 0, sizeof(data)) for all private data. And had resolved all of my issues related with wrapper.
As I understand wrapper mostly use with allocation in global memory which was initialized with zero.

Currently i use this wrapper and it covers all my needs.

Thanks.
User avatar
Giovanni
Site Admin
Posts: 14891
Joined: Wed May 27, 2009 8:48 am
Has thanked: 1202 times
Been thanked: 996 times

Re: C++ wrappers

Post by Giovanni »

In ChibiOS you must never rely on allocated memory to be zeroed, clearing it explicitly is the correct thing to do.

Just a note, clearing a virtual_timer_t structure is sufficient as initialization.

Giovanni
helmut
Posts: 18
Joined: Thu Apr 07, 2016 9:06 am
Has thanked: 1 time
Been thanked: 3 times

Re: C++ wrappers

Post by helmut »

Hi,

I'm through with my first attempt to C++-ify the cpp wrappers. There are still some issues we
need to discuss concerning dynamic memory. For now we replace malloc() and free() (and their
C++ pendants to use chHeapAlloc() on the default heap).

Please find attached the patch for discussion (using git format).
Helmut
Attachments
0001-improve-cpp-wrappers.patch.zip
(15.3 KiB) Downloaded 492 times
helmut
Posts: 18
Joined: Thu Apr 07, 2016 9:06 am
Has thanked: 1 time
Been thanked: 3 times

Re: C++ wrappers

Post by helmut »

HI,

just found a major quirk in the design of my ThreadReference.
Moving the ThreadStayPoint stuff to it drastically interferes with the inheritance
of Thread. One would overwrite the thread reference of a Thread by using suspendS() on it,
which of course was not the intention.

Helmut
User avatar
Giovanni
Site Admin
Posts: 14891
Joined: Wed May 27, 2009 8:48 am
Has thanked: 1202 times
Been thanked: 996 times

Re: C++ wrappers

Post by Giovanni »

Hi,

Thread References are synchronization objects and should be used as such.

Giovanni
helmut
Posts: 18
Joined: Thu Apr 07, 2016 9:06 am
Has thanked: 1 time
Been thanked: 3 times

Re: C++ wrappers

Post by helmut »

Hi

I'm through with the C++ wrappers, attached please find a new patch (from a relatively new
stable_16.1.x). I reworked most of the thread interface and moved to C++11. A few goodies are:

1) nearly no overhead to the C-version (my C-build had 36800 bytes (with the testers), the C++ one has 36912 bytes),
including __cxa_guard stuff for thread safe static initialization (based on osal and not ChibiOS mutexes).
2) some protection against typical C++ usage errors in the precincts of copyable, moveable, more type safety
in template classes (alignment in ObjectPool).
3) no exceptions requirement (the support for exceptions may not be complete yet)
4) no dynamic memory management requirement

Currently this is only tested for gcc version 5.2.1, but should work for earlier versions.
Please review and comment.

Helmut
Attachments
cpp_wrapper.patch.gz
(21.22 KiB) Downloaded 487 times
User avatar
Giovanni
Site Admin
Posts: 14891
Joined: Wed May 27, 2009 8:48 am
Has thanked: 1202 times
Been thanked: 996 times

Re: C++ wrappers

Post by Giovanni »

Hi,

I am a bit busy with other things (flash driver), any feedback on this? I see several downloads.

Giovanni
helmut
Posts: 18
Joined: Thu Apr 07, 2016 9:06 am
Has thanked: 1 time
Been thanked: 3 times

Re: C++ wrappers

Post by helmut »

Hi,

any progress with the wrappers? I don't want that to fizzle out, it was quite some work.
If I can do something to allow the wrappers be part of the next ChibiOS version I'm prepared
to take the effort.
As I mentioned before we'll go further and support exception handling in the next step.

Helmut
User avatar
Giovanni
Site Admin
Posts: 14891
Joined: Wed May 27, 2009 8:48 am
Has thanked: 1202 times
Been thanked: 996 times

Re: C++ wrappers

Post by Giovanni »

Few comments from me after a brief inspection:

1) The patch does not apply entirely on the current trunk, tortoise reports fail on several files.
2) Why does the patch modifies unrelated files like shell?
3) The patch mixes spaces are tabs, tabs are forbidden. Formatting is lost in several places.
4) Why use OSAL? RT does not include necessarily OSAL which is part of HAL.
5) Streams are no more part of RT, should be dropped.
6) What that non_zero() macro does?
7) // is forbidden by the style guideline.
8) TRUE/FALSE now are used only for conditional switches, use true/false in code.
9) Comments tabulation points are lost after renaming of parameters.
10) Exceptions should never be required.

Giovanni
helmut
Posts: 18
Joined: Thu Apr 07, 2016 9:06 am
Has thanked: 1 time
Been thanked: 3 times

Re: C++ wrappers

Post by helmut »

Hi Jovanni,

thanx for review.

1) this is quite obvious, as I was changing against stable_16.1.x dated 10.4.2016, I'll rebase to any version you define!
Your trunk is a somewhat 'moving target', maybe you can give me a dedicated version (best from the github mirror).
2) shell.c changes are minimal, I modified it because our local tester uses the c++-wrappers to start the shell
This is not visible to you, sorry. I'll update the RT-STM32F407-DISCOVER-G++ to also start the shell, so it will be obvious why I need the change,
unless you find something more suitable:

Code: Select all

// Shell manager initialization
shellInit();
/*
 * Normal main() thread activity, in this demo it does nothing except
 * sleeping in a loop and check the button state, when the button is
 * pressed the test procedure is launched.
 */
while (true) {
  Ch::Thread shell(2048, shell_thread, (void *) &shell_cfg1, Ch::Thread::currentPriority()+1);
  shell.wait();
}

3) I'll fix this, your coding style is not too friendly to me ;)
4) OSAL is only used in the cxa_guard stuff. This is a multistage problem:
a) cxa_guard is necessary to support low level c++ (no dynamic memory, no RTOS)
b) syscalls_cpp.cpp is kind of mid level c++ (dynamic memory, only chHeap... is currently used, maybe that could be no RTOS aswell)
c) ch.hpp is high level c++ (dynamic memory and RTOS)
That is why I tried to stay at OSAL level for the cxa_guard stuff. Maybe even the directory structure needs a change to reflect this!
5) ok, I'll look into that
6) the non-zero() function is a zero overhead helper, that allows you to create chDbgAssert() calls if the thread reference is zero,
if debug is disabled the compiler will simply optimize it out.
7) this is kind of strange for C++, but ok, I'll change.
8) this is only in the documentation, which stems from the ChibiOS C-call documentation, which changed afterwards obviously, I'll change
9) sorry, I don't understand this, please be more specific!
10) exceptions are definitely needed by us, I promise they'll not create any overhead if not used ;-), I just fear that future modifications
of the C++ support code will break our exceptions related stuff if it's not contributed.

Helmut
Post Reply