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

Using Tinq for ROS 2.0 #7

Open
vmayoral opened this issue Aug 6, 2014 · 178 comments
Open

Using Tinq for ROS 2.0 #7

vmayoral opened this issue Aug 6, 2014 · 178 comments

Comments

@vmayoral
Copy link

vmayoral commented Aug 6, 2014

@brunodebus and @mouse256, trying to use this way to reach you (e-mail didn't work so far):

This is Víctor Mayoral Vilches, working at OSRF. Esteve contacted you a while ago showing our interest in Qeo and Tinq. I am the person pushing forward the ROS 2.0 development on the embedded/deep-embedded side and we are considering using Tinq's code for this purpose.

Our goal is to use a minimal DDS implementation in plain C (ideally ANSI C11) that can run in bare-metal (cortex-m3, -m4, -m0, msp430, ...) so we can't assume a POSIX interface.

So far we have the following architecture in mind:

  • The Transport abstraction layer will hook up the embedded network stack (probably device dependant) with the DDS.
  • We are hoping that the DCSP and RTPS layers can be provided by the tinq-core will provide once it has been stripped down from the POSIX networking and RT dependencies.
  • The ROS client nano library API will abstract the users with a rossified API of the underlying DDS.

How does this sound to you? Would it be fine if we contributed to tinq-core to make it deep-embedded capable? If so, maybe you could clarify to me the following aspects:

  • As tinq-core is coded right now, it assumes it's built on top of POSIX, right? How feasible is it to remove this dependency and provide an interface that allows the networking stack to be adaptable? (POSIX could be one option)
  • I see that currently, libdds.so is 3.5 MB. We are targeting something around 200 KB. Do you believe it's possible stripping it down to this size?
  • Is the real-time implementation depending on librt and if so, could this be bypassed easily?

After having spent some time looking at the code I admit that it's a quite amazing piece of work. I think it'll be great to use Tinq as a fundamental piece of software for embedded robotics.

@vmayoral vmayoral mentioned this issue Aug 6, 2014
@bramstes
Copy link

bramstes commented Aug 6, 2014

Hello,
I'm going to try to get you in touch with another member of the original Qeo development team he's the main developer/designer behind the DDS layer and will be able to provide you with the correct answers. He's currently enjoying some well deserved holidays but I'll inform him and ask to join.

@bramstes
Copy link

bramstes commented Aug 6, 2014

The plain C requirement should be ok but there're dependencies on POSIX although things like thread, lock, socket, memory ... creation/handling/management have their own abstraction layers inside the tinq/qeo dds code (with implementations for linux, windows, possibly mac)
With regards to size it will depend on which features you will need it was originally designed to be able to run on dsl gateways and stb'es. 200KB might be ambitious but our colleague will be able to tell more.

@vmayoral
Copy link
Author

vmayoral commented Aug 6, 2014

Hey @bramstes thanks for the quick answer. We are really excited about the chances of using Tinq. Although we'd like to make it as deep-embedded as possible we are not so strict about the 200 KB. We probably can get started with a less ambitious aim. Let's say 500 KB.

It'll be important to identify which are the POSIX dependencies and how could we bypass them into minimal implementations. It's specially important to make sure that RTPS is working without this dependencies.

I'll look forward for your input. Meanwhile i keep diving into your code.

@brunodebus
Copy link
Owner

Guys, I'm currently on holidays with limited access to the internet...(this
and next week). But I like the idea of trying to get a subset of the code
running on more constrained devices... I will try to look at the proposal
ASAP...
Op 6-aug.-2014 20:35 schreef "bramstes" notifications@github.com:

The plain C requirement should be ok but there're dependencies on POSIX
although things like thread, lock, socket, memory ...
creation/handling/management have their own abstraction layers inside the
tinq/qeo dds code (with implementations for linux, windows, possibly mac)
With regards to size it will depend on which features you will need it was
originally designed to be able to run on dsl gateways and stb'es. 200KB
might be ambitious but our colleague will be able to tell more.


Reply to this email directly or view it on GitHub
#7 (comment).

@kriver
Copy link

kriver commented Aug 6, 2014

With the default setup on my 64-bit Linux libdds.so measures up to 4.4Mb unstripped, but only 823Kb stripped. When disabling security it goes down to 3.3Mb and 634Kb respectively. So depending on what you want in or out a goal of 500Kb may be feasable. (Haven't build ARM binaries yet.)

@jvoe
Copy link

jvoe commented Aug 7, 2014

Hi guys.
As Bram already mentioned, we don't really have Posix-specific dependencies. The main dependencies on OS functions are all abstracted (see dds/src/co/*). Even threading/locking can be mapped to something other than pthreads. Actually, I once ported the DDS part to the Windows OS, using native Windows functions for every OS primitive that was required.
An initial version of DDS didn't even use threading at all, but I think it would be difficult to get that variant running again.
As to librt we only use the clock_gettime() function.
The main reason why libdds is quite big is due to the fact that a lot of functionalities are taken along by default: the security framework, the X-types framework, and a lot of other stuff. Security can be stripped out completely, X-types can be replaced with the original type library, and other optimizations can be done by removing specific compile-time flags (removing a lot of tracing/debugging, fragmentation support, typecode exchange support, etc).
I'll be glad to help if you have specific questions on how to do all this.

@vmayoral
Copy link
Author

vmayoral commented Aug 8, 2014

@jvoe @kriver @brunodebus and @bramstes, thanks a lot for your inputs.

Let me summarize the conclusions stated so far:

  • The size limitation should not necessarily be an issue. @kriver already informed about 634 Kb (without security) and we probably can further strip it down.
  • All OS dependencies (RT and networking) are abstracted under dds/src/co/ which makes it easier to port it to a different platform.

According to this facts, moving forward with Tinq's dds code seems a great solution. My next step will be to create a minimum prototype with FreeRTOS as the underlaying system.

I've selected FreeRTOS for it's popularity, weird/relaxed GPL license, and because of its portability. FreeRTOS has ports for many devices and is extremely lightweight.

The prototype should include:

dds chat
tinq dds
FreeRTOS

@jvoe
Copy link

jvoe commented Aug 9, 2014

@vmayoral,
I had a quick look at FreeRTOS and i see a few issues porting Tinq DDS to it:
The first issue is a licensing one. Standard FreeRTOS doesn't have UDP support, unless the FreeRTOS+UDP package is used. This one is freely downloadable but has a different licensing model from the vanilla FreeRTOS license. The latter (FreeRTOS) relaxes the GPL license so that other software (commercial or other) can be added on top of it without having to be disclosed. The former (FreeRTOS+UDP), however, does enforce a pure GPLv2 so that all software linked to it becomes GPLv2. I don't know whether this is a problem for ROS 2.0?
Other networking components (TCP, TELNET, HTTP, FTP, etc) are not free and require a hefty fee to be used. Luckily, TinqDDS only requires UDP to function if only DDS is to be used.
The other issue i see is that the FreeRTOS+UDP software doesn't include poll() support, i.e. only select() is implemented. This would require a change to the dds/src/co/sock.c since it is currently assumed that poll() is available on target systems. Not a big issue since the rest of DDS uses the functions in sock.c and this change is quite easy to do.
Anyway, just my 2 cents ..
Jan Voet

@vmayoral
Copy link
Author

vmayoral commented Aug 9, 2014

Hey @jvoe! Thanks a lot for the follow up.

With regard the licensing aspect, as you point out, at the OSRF we can't accept a GPL. FreeRTOS offers a relaxed GPL and after reviewing it we consider that it should be sufficient for our users. Regarding the networking stack, you're right. We won't be using the default FreeRTOS+UDP but lwIP which has a nice BSD license and is easily integrated in FreeRTOS.

I haven't fully dived yet on lwIP yet but i saw that it has a udp_recv function that allows to pass a callbac. I'd say that this function is implemented through polling so this might satisfy the polling requirement that you mentioned.

Anyhow it's really great hearing that the interfaces are hackable :). I'm coding a first prototype at https://github.com/vmayoral/ros2_embedded. Hopefully next week i'll come back with feedback.

@vmayoral
Copy link
Author

@jvoe @kriver @brunodebus and @bramstes, after the having spent the last week working on freeRTOS (that provides a minimal layer of pseudo-threads called tasks) and a minimal UDP implementation on top of it (coded by us, we discarded lwIP) we are ready to start porting the Tinq's interfaces to our embedded target (STM32F4).

Since it's a non-POSIX environment (C99) i've spent some time trying to figure out (besides dds/src/co that @jvoe already mentioned) what should be ported. So far i've concluded that:

  • Everything under dds/src/co should be coded for the embedded target as pointed out by @jvoe
  • the dds/src/trans should also be specialized for the target
  • dds/src/dynip

A few questions if you allow me:

  • Does Tinq rely heavily on the sql (dds/src/sql) or can we just drop it from the compilation. Will this break Tinq?
  • How relevant is the dds/src/co/sock.c code? It seems to me like a scheduler interface.
  • How many threads does approximately Tinq's DDS implementation require?

@jvoe
Copy link

jvoe commented Aug 21, 2014

@vmayoral As to your questions:

  • The src/sql directory contains the bytecode compiler/interpreter for SQL-like filter expressions as used in DDS FilteredTopics as well as DDS QueryConditions. If you don't use filters it could be removed from the source code, I guess. But I would check first with the size tool to see how big this piece of code really is, before trying to remove it and all the code around it. It might lead to a lot of work for a relatively small gain.
  • the sock.c is a small layer on top of the file descriptor and socket code of an OS, abstracting the Posix poll() or select() functionality. The rest of the code uses this layer to dynamically add/remove file handles and sockets to the poll() (or select()) set of file descriptors. The poll()/select() functionality is checked from the core thread of DDS (see src/dds/dds.c) where the real scheduling occurs. In there, time-outs are checked as well as scheduling the rest of the internal DDS tasks. When there is nothing to do anymore, the sock poll()/select() functionality is called with an argument (derived from the list of active timers) specifying how long this function may block, waiting for file/socket events. If there are ready file descriptors (read/write/connect/accept), this is indicated as an internal DDS event bit. If there are DDS events detected, these are then processed by calling relevant internal DDS functions.
  • The current DDS implementation uses only 2 threads. The core thread (as described before) and the application thread (when the process started).
  • The DynIP layer is responsible for determining IP address changes and IP interfaces going down/up. On Linux machines, this abstracts the netlink functionality. On Windows/OSX/iOS/BSD, other mechanisms can be used.
  • For src/trans, I would simply not enable the non-relevant blocks by not specifying compile-time options. Examples: DDS_IPV6 determines support for IPv6, DDS_SECURITY and DDS_NATIVE_SECURITY determine whether you want the software security enabled (DTLS or TLS). DDS_TCP for support of TCP, etc. Check the Makefile of the DDS library, or even the apps/* Makefiles, where for the latter various sets of compile time options are used. It would be a lot of work to rewrite the whole IP interfacing infrastructure, I'm afraid. Making Posix-like read/write() variants on top of your IP networking code shouldn't be too hard.
  • A lot of functionality in dds/src/co is generic and doesn't need to be ported. Some are very system-specific however, such as sys.[ch], sock.[ch], thread.[ch], ipc.[ch]. Easiest approach would be adding code surrounded by #ifdefs, as already done to differentiate between Linux/OSX/iOS/Android/FreeBSD/NetBSD/Windows versions.

Let me know how it's going. This is exciting work and I hope you don't encounter too many difficulties.
I'll be glad to answer more questions as you proceed.

@vmayoral
Copy link
Author

@jvoe @kriver @brunodebus and @bramstes,

After struggling some time with FreeRTOS and its low level complexities we decided to take a step back and evaluate other RTOS options so we spent the last weeks putting together three prototypes:

After having evaluated the three of them we decided to select NuttX for three main reasons:

  • POSIX (pseudo) interface
  • BSD socket interface
  • Well documented and supported

I'm working now on full speed at https://github.com/vmayoral/ros2_embedded_nuttx which looks quite promising (specially interesting is the NuttX shell NSH which can be hacked to include DDS inspecting capabilities).

I ported sys and sock so far and I'm working on ipc. Semphores is something we will need indeed in the embedded world however we are not so sure about the shared memory functionality. In general purpose operative systems we understand that several processes sharing memory can lead to a more efficient performance however as we conceive ROS 2.0 for embedded systems, we can't think of an scenario requiring it.

Is shared memory a big pilar within Tinq's code? Is there an alternative way of interprocesses communication implemented (maybe a slower one?).

Would love to hear your opinion in this matter.

@bramstes
Copy link

Typically most of the time we used unix domain sockets to do ipc on our gateways but jvoe and Bruno are better placed to answer what is supported in dds (i remember shared memory is something we discussed at some point a.o.in the context of avoiding discovery info dupplication but don't remember us actually implementing it).Since you say the BSD socket interface is supported on nuttx, Unix domain sockets could be an option. But like I said I'd prefer jvoe and/or Bruno to answer :-).CheersBram.-------- Oorspronkelijk bericht --------Onderwerp: Re: [tinq-core] Using Tinq for ROS 2.0 (#7)Van: Víctor Mayoral Vilches Aan: brunodebus/tinq-core Cc: bramstes @jvoe @kriver @brunodebus and @bramstes,

After struggling some time with FreeRTOS and its low level complexities we decided to take a step back and evaluate other RTOS options so we spent the last weeks putting together three prototypes:

ros2_embedded_nuttx
ros2_embedded_freertos
ros2_embedded_riot
After having evaluated the three of them we decided to select NuttX for three main reasons:

POSIX (pseudo) interface
BSD socket interface
Well documented and supported
I'm working now on full speed at https://github.com/vmayoral/ros2_embedded_nuttx which looks quite promising (specially interesting is the NuttX shell NSH which can be hacked to include DDS inspecting capabilities).

I ported sys and sock so far and I'm working on ipc. Semphores is something we will need indeed in the embedded world however we are not so sure about the shared memory functionality. In general purpose operative systems we understand that several processes sharing memory can lead to a more efficient performance however as we conceive ROS 2.0 for embedded systems, we can't think of an scenario requiring it.

Is shared memory a big pilar within Tinq's code? Is there an alternative way of interprocesses communication implemented (maybe a slower one?).

Would love to hear your opinion in this matter.

—Reply to this email directly or view it on GitHub.

@brunodebus
Copy link
Owner

Hi,

IPC is only used to make sure multiple dds instances on the same system get a unique participant id. The partiticipant id is part of the guid (it makes the guid unique on one system) and it is used to choose which ports will be used by this participant for communication.

You can compile the code with the define NOIPC to turn of this code (we use this on android where we also cannot use posix ipc). Basically, the code will also work, but not as efficient: it generates a random partiticipant id, and tries to open the corresponding ports. If that fails it will restart and try the next participant id.

So you can try with NOIPC first. If you will ever run a lot of participants on the same device ( not the idea I think :-) ) we can figure something else out.

@vmayoral
Copy link
Author

@brunodebus and @bramstes,

Thanks for your answers. Trying it with -DNOPIC for now seems like a great idea since we want to get a prototype running ASAP.

I'm fighting a bit with the security implemented in DDS. My current configuration is available here. Is there any other flag that you recommend to remove(or add)?

Since security is nicely integrated in the code and there's no official support for openssl in NuttX i've decided to omit security to the maximum point for now. I'm hand excluding code like:

#include "openssl/ssl.h"
...
#include "dds/dds_security.h"

Let me know if you have any comments on this matter please.

@vmayoral
Copy link
Author

Instead of continuing that path which was a bit senseless because it was breaking all of your work i just droped sec_CSRCS and splug_CSRCS from the Makefile (ros2/ros2_embedded_nuttx@4d65f25).

@vmayoral
Copy link
Author

@jvoe @kriver @brunodebus and @bramstes,

It seems we are getting way closer 👍. The current prototype with NuttX is providing pretty good results on the integration side. Let me share with you my two current blockers:

  • As discussed by @jvoe before, in NuttX there's also the problem of the poll()/select() primitives that are not implemented for UDP. This is the point in the code to be addressed. Speaking with @NuttX he suggested these two options:
  • Implement UDP packet buffering, or
  • Create your one UDP listen thread that waits of recvfrom() all of the time and buffers UDP packets in memory where they can be read and informs some other thread that the data is available (perhaps via a signal).

I'd say it shouldn't be too hard to code something here that uses recvfrom() calls, stores the data in some buffer and returns as soon as there's data available. Could you advise if this is the best way to fix this issue?

  • The size of the binary is just too big. We are currently these flags to compile Tinq's DDS.

Currently the linker is complaining (i'm trying to build the DDS chat example):

arm-none-eabi-ld: /home/victor/Dropbox/OSRF/ros2_embedded_nuttx/nuttx/nuttx section .bss' will not fit in regionsram'
arm-none-eabi-ld: region `sram' overflowed by 61400 bytes

The board we are using has 112KiB + 16 KiB of usable SRAM. @NuttX helped us understand that :

The error message above means that your program static data is currently 112KiB + 60KiB or about >182KiB. That does not fit into any available memory region. So you are stopped until you solve that >problem. But then you are going to have another big time memory usage program when your program >starts.

We definitely need to shrink the code size quite a bit if we wish to continue moving forward (specially because we also will need to create a ROS thin layer on top of DDS). Can you please provide suggestions on what we could remove? For now a minimal DDS implementation with RTPS should do it.

Once again, we are really happy and thankful for your support. Thanks everyone!

@vmayoral
Copy link
Author

I removed -DEXTRA_STATS and -DCDD_USED but the size didn't decrease. I will try to remove next DDDS_DEBUG but a quick test suggest that some work fixing dependencies will be needed. Seems the same for XTYPES (actually it looks pretty hard to remove XTYPES if even possible)

@brunodebus
Copy link
Owner

About the memory issue: actually, code size is not the reason for the linking error you are getting, .bss (zero initialized data is). The main culprits for this are the send and receive buffers allocated in dds/src/trans/ip/rtps_ip.c (lines 157 & 172). They are each ~64kB.... (MAX_RX_SIZE/MAX_TX_SIZE)

You can change this size (and you will only be able to receive smaller messages) in ./dds/src/include/ri_data.h and your program should be able to link.

@jvoe: Does the buffer need to have a specific size/alignment? I cannot remember.

@vmayoral
Copy link
Author

@brunodebus thanks for the suggestion. That did it.

@bramstes, @brunodebus, @kriver and @jvoe: I'm trying now to run the chat on the embedded board but it seems it might need quite a bit of work. Right now, i'm trying to figure out the right configuration for the ParVal_t parameters in NuttX. Usually they are loaded from a file but in this case NuttX has no FS (not that we are using at least).

My idea is to set them by default to a reasonable value in the NuttX case. Could you provide some reference on what values these parameters should take? (dds/doc/tdds-env.doc seems empty). Or maybe a set of tips on what values these params should take to be able to run in a embedded system?

@gregory-nutt
Copy link

I would suggest that you use the configdata interface to hide the
implementation of where configuration parameters are stored (or even if they
are hardcoded). See for example apps/examples/configdata.

NuttX, of course, does support a variety of file systems. The STM32F4DIS-BB
board has a microSD slot on board that is fully supported by NuttX... you just
have to enable it. It is already enabled in the
configs/stm32f4discovery/netnsh configuration.

Traditionally, embedded systems keep configuration data in tiny serial FLASH or
EEPROM. Most network-enabled embedded systems needed some configuration storage
for, at a minimum, the MAC address of the device. A small serial storage device
is usually all that is required.

Hardcoding anything is generally a bad idea. But hiding the source of
configuration data behind the configdata interface at least makes what you
planning portable.

Greg

On September 24, 2014 at 5:19 PM Víctor Mayoral Vilches
notifications@github.com wrote:

@brunodebus https://github.com/brunodebus thanks for the suggestion. That
did it.

@bramstes https://github.com/bramstes , @brunodebus
https://github.com/brunodebus , @kriver https://github.com/kriver and
@jvoe https://github.com/jvoe : I'm trying now to run the chat on the
embedded board but it seems it might need quite a bit of work. Right now, i'm
trying to figure out the right configuration for the ParVal_t parameters
https://github.com/vmayoral/ros2_embedded_nuttx/blob/old-code/dds/src/co/config.c#L71
in NuttX. Usually they are loaded from a file but in this case NuttX has no FS
(not that we are using at least).

My idea is to set them by default to a reasonable value in the NuttX case.
Could you provide some reference on what values these parameters should take?
(dds/doc/tdds-env.doc seems empty). Or maybe a set of tips on what values
these params
https://github.com/vmayoral/ros2_embedded_nuttx/blob/old-code/dds/src/co/config.c#L71
should take to be able to run in a embedded system?


Reply to this email directly or view it on GitHub
#7 (comment) .

@jvoe
Copy link

jvoe commented Sep 25, 2014

@vmayoral: for documentation on DDS configuration, as well as for other specifics of Tinq DDS, check the dds/doc/dita/out/user_manual.pdf.
The XTypes type system can be replaced with an older variant of type handling that only supports TSM-based type definitions. This might save on some code/data. To do this, undefine XTYPES_USED from the Makefile, remove all src/xtypes/.[ch] files from CSRCS, and add the src/typecode/.[ch] files instead to CSRCS (pid.c, cdr.c, pl_cdr.c and typecode.c). This probably will require a number of changes to the rest of DDS though, since a few parameters have been added to the type functions since the XTypes mechanism was introduced. However, it should be possible with a bit of work/patience.
@brunodebus: I don't think there is an alignment restriction to the buffer sizes. I wouldn't go lower than 1500 though (the Ethernet frame size), and keep it a multiple of 4, just to be safe :-).

@vmayoral
Copy link
Author

@jvoe thanks for your answer. I was hoping i could get a link to how things get configured for a sample application. There're many parameters that although well documented, i don't feel familiar enough with the DDS implementation to guess a good value. For example:

    { G_Pool, DC_Pool_Subscribers,      "SUBSCRIBERS",   V_Range,  0, NULL, {0}},
    { G_Pool, DC_Pool_Publishers,       "PUBLISHERS",    V_Range,  0, NULL, {0}},
    { G_Pool, DC_Pool_Readers,          "READERS",       V_Range,  0, NULL, {0}},
    { G_Pool, DC_Pool_Writers,          "WRITERS",       V_Range,  0, NULL, {0}},
    { G_Pool, DC_Pool_Topics,           "TOPICS",        V_Range,  0, NULL, {0}},
...

It will be great if you could provide a pointer to how things are configured for the chat application which is the one i'm trying to build for now.

@jvoe
Copy link

jvoe commented Sep 26, 2014

@vmayoral For the minimal amount of memory on startup, use -DFORCE_MALLOC. This will force all required memory to be malloc-ed from the heap instead of using static memory pools. Your application will only use what it needs then. There will be fragmentation overhead, of course, and this is not as fast as memory pools, but these disadvantages might not be that bad.
On the other hand, if you really need things to be configured up front, thereby avoiding dynamic allocations altogether, the DDS_get_default_pool_constraints() and DDS_set_pool_constraints() functions allow you to do this very easily.
To find out your actual pool requirements, run the application in its worst case usage, and check the really used memory amount with the "spool" DDS debug command.

@vmayoral
Copy link
Author

@NuttX and @LorenzMeier,
we are having some issues while running our application and we discovered that there's a portion of the FLASH memory that is just not accessible. Have you seen something similar? ros2/ros2_embedded_nuttx#7.

@vmayoral
Copy link
Author

The issue with the memory has been fixed. It seems like stlink had a bug that didn't allow to debug all the memory (1 MB) available in the STM32F4Discovery. Refer to ros2/stlink@d855279 for the fix.

Keep moving forward. @jvoe will try setting up the DDS Debug shell as soon as possible.

@vmayoral
Copy link
Author

@bramstes, @brunodebus, @kriver, @NuttX and @jvoe: I finally got the chat example application working. After some fixes that allowed the serial UART to use the prompt i encountered the following issues while trying to interoperate the board with chat running in a Desktop machine (Ubuntu):

Any comments will be appreciated.

@gregory-nutt
Copy link

A quick hack has been applied to one of the mutexes in the code.
It was causing the system to hang and it didn't seem trivial to fix.
Refer to ros2/ros2_embedded_nuttx#8. This hack allows the
application to run.

I don't understand what the issue is

Apparently, it's not possible to validate the parameters without
causing the system to crash. Refer to this issue
ros2/ros2_embedded_nuttx#10

Looks like a stack overrun. This is very close to overflowing the
stack. My guess is that probably has overflowed the stack in the past:

sp: 10003090
stack base: 10003750
stack size: 000007e4

Definitely overran the stack:

sp: 10002f48
stack base: 10003750
stack size: 000007e4
ERROR: Stack pointer is not within the allocated stack

You will need to increase the stack size. Overrunning the stack can cause all
kinds of crazy, inexplicable behaviors.

Greg

@jvoe
Copy link

jvoe commented Nov 11, 2014

@vmayoral So an unmarshalling problem after all. Probably in one of the subfunctions of pid_parse(), called from pid_parse_reader_data() or from pid_parse_writer_data(). Try stepping into that pid_parse() with a first breakpoint in one of the caller functions, examining the parameter_id value each loop iteration until you see it failing. The subfunction that parses that parameter_id can get a breakpoint next to see where it goes wrong.

@vmayoral
Copy link
Author

@jvoe debugged pid_parse() and here's what i've found out (apologies beforehand for a message full of captures):

The error 3 when unmarshalling always happens when walk->length is 4:
screenshot from 2014-11-11 10 47 15

The error always comes from this call:
screenshot from 2014-11-11 10 52 34

Which if I step in corresponds with pid_typecode_parse function:

screenshot from 2014-11-11 10 52 55

Particularly it seems that here's where the problem comes in:

screenshot from 2014-11-11 10 53 11

Not sure how to interpret this matter however in the embedded board i clearly get the messages complaining about an unmarshalling issue. The following picture also shows the package being sent from the Desktop DDS to the Embedded DDS which repeatedly happens (capture available here).

screenshot from 2014-11-11 11 08 00

@jvoe
Copy link

jvoe commented Nov 11, 2014

@vmayoral Looks like the issue is in parsing the typecode of the publication/subscription topic, which isn't the easiest piece of code. Fortunately the Chat topic is not a complex one, so this is doable.

Strange that this doesn't work well on the target as the same Chat topic is properly parsed on the desktop with no issues whatsoever. Also, this code was tested on a myriad of other target platforms, Little endian as well as Big endian, including MIPS CPUs, various ARM CPUs (RPi, iOS, lots of different Android phones/tablets), and 32-bit as well as 64-bit x86 architectures.

Just a thought, have you tried compiling with normal enums, i.e. 32-bit, as on the desktop? Please try that before continuing the bughunt, since this seems to be the only difference between your toolchain and the ones we use.

@vmayoral
Copy link
Author

@jvoe i added the flag -fno-short-enums hoping that this will do it. When doing this i get the following warnings thought:

arm-none-eabi-ld: warning: /home/victor/Dropbox/OSRF/ros2_embedded_nuttx/nuttx/lib/libapps.a(ghpringbuf.o) uses 32-bit enums yet the output is to use variable-size enums; use of enum values across objects may fail
arm-none-eabi-ld: warning: /home/victor/Dropbox/OSRF/ros2_embedded_nuttx/nuttx/lib/libapps.a(dds_seq.o) uses 32-bit enums yet the output is to use variable-size enums; use of enum values across objects may fail
arm-none-eabi-ld: warning: /home/victor/Dropbox/OSRF/ros2_embedded_nuttx/nuttx/lib/libapps.a(guard.o) uses 32-bit enums yet the output is to use variable-size enums; use of enum values across objects may fail
arm-none-eabi-ld: warning: /home/victor/Dropbox/OSRF/ros2_embedded_nuttx/nuttx/lib/libapps.a(dcps_dbg.o) uses 32-bit enums yet the output is to use variable-size enums; use of enum values across objects may fail
...

My guess is that this happens because the NuttX code does not have this flag enabled thereby they collide. @NuttX any idea on how i can force 32 bit enums within NuttX?

@gregory-nutt
Copy link

@vmayoral "any idea on how i can force 32 bit enums within NuttX?"

The CFLAGS come from your code. You set them in your Make.defs file

@LorenzMeier
Copy link

@vmayoral You might want to run 'make distclean' and ensure you don't have a stale build.

@vmayoral
Copy link
Author

@LorenzMeier and @NuttX thanks for your input. I make distclean-ed and added this commit ros2/ros2_embedded_nuttx@bacb554 which should include the -fno-short-enums in CFLAGS but i'm still the same warning from the linker.

@LorenzMeier
Copy link

Do you have the flags also in the board config, e.g. here in both lines?
https://github.com/ros2/ros2_embedded_nuttx/blob/master/nuttx/configs/stm32f4discovery/dds_standalone/Make.defs#L77-L78

If you use a different board config, then in that board config as well. You also need to reconfigure to bring those changes into the compile / staging area.

cd nuttx-git
cd nuttx
make distclean
cd tools
./configure.sh stm32f4discovery/dds_standalone
cd ..
make

@vmayoral
Copy link
Author

@LorenzMeier thanks! Adding the flag in the board configuration files did it.
Warnings now inform:

arm-none-eabi-ld: warning: /usr/lib/gcc/arm-none-eabi/4.8.2/armv7e-m/libgcc.a(bpabi.o) uses variable-size enums yet the output is to use 32-bit enums; use of enum values across objects may fail
arm-none-eabi-ld: warning: /usr/lib/gcc/arm-none-eabi/4.8.2/armv7e-m/libgcc.a(_divdi3.o) uses variable-size enums yet the output is to use 32-bit enums; use of enum values across objects may fail
arm-none-eabi-ld: warning: /usr/lib/gcc/arm-none-eabi/4.8.2/armv7e-m/libgcc.a(_udivdi3.o) uses variable-size enums yet the output is to use 32-bit enums; use of enum values across objects may fail

Tried flashing the board with the new firmware and i still get the same unmarshalling problem:

dds_get_cdata: error 3 unmarshalling PL-CDR data (writer=29)!
disc_get_data({9}) returned error 3!

I'll keep looking into it.

@jvoe
Copy link

jvoe commented Nov 11, 2014

@vmayoral Sorry to hear it didn't help. You seem to be very close to successful communication though.

As a drastic workaround, could you try removing -DDDS_TYPECODE from the chat Makefile? Don't know if it will still compile properly, but it effectively switches off the sending/verifying of type information in topics. If it compiles you'll have to do it both on the Desktop and on the board.

@vmayoral
Copy link
Author

that did it @jvoe! hurray!
screenshot from 2014-11-11 15 03 51

@jvoe
Copy link

jvoe commented Nov 11, 2014

@vmayoral Nice work 👍 Well done!

@vmayoral
Copy link
Author

@NuttX I'm trying to come up with a simple DDSIMU demo and for that purpose i'm trying to use one of the I2C accelerometers in the board.

After having made sure that the sensor is properly recognized (using NSH i2c tool which is great) i coded a simple program to configure the sensor and read from it. I successfully read the registers properly however the write_register function doesn't seem to work:

Hello, IMU!!
I2C Successfully initialized
WHO_AM_I register = 0x33 (51)
***********************************
REGISTERS:
***********************************
register 0x20 = 0x07 (7)
LIS302_CTRL_REG1 (0x20) register = 0x07 (7)
register 0x21 = 0x00 (0)
LIS302_CTRL_REG2 (0x21) register = 0x00 (0)
register 0x22 = 0x00 (0)
LIS302_CTRL_REG3 (0x22) register = 0x00 (0)
register 0x23 = 0x00 (0)
LIS302_CTRL_REG4 (0x23) register = 0x00 (0)
register 0x24 = 0x00 (0)
LIS302_CTRL_REG5 (0x24) register = 0x00 (0)
***********************************
I2C Successfully initialized
writting 0x27 (39) in register 0x20
***********************************
REGISTERS:
***********************************
register 0x20 = 0x07 (7)
LIS302_CTRL_REG1 (0x20) register = 0x07 (7)
register 0x21 = 0x00 (0)
LIS302_CTRL_REG2 (0x21) register = 0x00 (0)
register 0x22 = 0x00 (0)
LIS302_CTRL_REG3 (0x22) register = 0x00 (0)
register 0x23 = 0x00 (0)
LIS302_CTRL_REG4 (0x23) register = 0x00 (0)
register 0x24 = 0x00 (0)
LIS302_CTRL_REG5 (0x24) register = 0x00 (0)
***********************************

Could you advise if there's something wrong with my implementation or the way that i'm using the I2C API?

@gregory-nutt
Copy link

@vmayoral I can pretty much guarantee you that there is nothing wrong with the I2C write function. You will need to study the data sheet for the part to assure that you are communicating with it according its specification. Almost certainly you are not.

For example, this is probably wrong:

result = I2C_WRITE(i2c, &reg_val, 1);
...
result = I2C_WRITE(i2c, &value, 1);

Is most likely wrong because it will cause a repeated START. Maybe you need:

uint8_t data[2];
data[0] = reg_val;
data[1] = value;
result = I2C_WRITE(i2c, data, 2);

Read the data sheet!

@gregory-nutt
Copy link

@vmayoral By the way, some devices require the repeated start condition. That is why it is impossible to look at code and determine if it is correct or not. The normal way to debug I2C is with a logic analyzer: You have to capture the waveforms that you generate and then compare than with the device's data sheet. The Seleae logic analyzer does a great job with I2C and is highly recommended.

@LorenzMeier
Copy link

@vmayoral This probe is really, really good. I recommend to get one: https://www.saleae.com/logic

@vmayoral
Copy link
Author

@NuttX and @LorenzMeier thanks to both. Will do.

@vmayoral
Copy link
Author

We've made some tests (including @NuttX free implementation of NuttX shell within the DDS Debug shell) and we got the following results when it comes to RAM requirements:

              total       used       free    largest
Data:       2175200     107680    2067520    2066704
Prog:        524288     524288          0          0

which incurs in about 631968 Bytes of SRAM. @jvoe does this sound reasonable to you?

@vmayoral
Copy link
Author

I checked (script) the RAM memory consumption of Tinq running in Desktop (with security and many other functions active) and i

1.5 MiB chat0

@vmayoral
Copy link
Author

In order to verify this matter we played with the parameter and figure out how much extra memory is needed we modified CONFIG_HEAP2_SIZE:

262 144 bytes

With a CONFIG_HEAP2_SIZE=262144 it does work fine:

mm_initialize: Heap: start=2000cf14 size=78060
mm_addregion: Region 1: base=2000cf14 size=78048
mm_addregion: Region 2: base=64000000 size=262144
....

131 072 bytes

A CONFIG_HEAP2_SIZE=131072 also seems to be sufficient. A simple pub/sub program works fine and the memory regions are:

mm_initialize: Heap: start=2000cf14 size=78060
mm_addregion: Region 1: base=2000cf14 size=78048
mm_addregion: Region 2: base=64000000 size=131072

65 536 bytes

CONFIG_HEAP2_SIZE=65536 takes a few seconds to get started. The program gets stucked for a few seconds after printing the memory regions but it eventually works:

ABCDF
mm_initialize: Heap: start=2000cf14 size=78060
mm_addregion: Region 1: base=2000cf14 size=78048
mm_addregion: Region 2: base=64000000 size=65536
Network configured, starting DDS chat:
List pools created.
String pool initialized.
Typecode pools created.
Typesupport initialized.
Socket handler initialized.
Timer pool initialized.
Data buffer pools created.
Random host identifier generated.
Unique GUID prefix created: 9679b4d9:00000002:00060000
Locator pools created.
History cache pools created.
Entities: reserved=98, maximum=4294967295
Domain pools created.
RTPS Initialised.
IP: scope = link..global
IP interfaces:
    eth0     : 192.168.0.3
RTPS over IPv4 Initialised.
...

32 768 bytes

Same behaviour, it slowly starts but i can see the DDS packages being sent over the network nicely.

16 384 bytes and below

There's not enough memory and it's not possible to dynamically instantiate the ChatMsg with xtypes:

ABCDF
mm_initialize: Heap: start=2000cf14 size=78060
mm_addregion: Region 1: base=2000cf14 size=78048
mm_addregion: Region 2: base=64000000 size=16384
Network configured, starting DDS chat:
List pools created.
String pool initialized.
Typecode pools created.
Typesupport initialized.
Socket handler initialized.
Timer pool initialized.
Data buffer pools created.
Random host identifier generated.
Unique GUID prefix created: 9761b4d9:00000002:00060000
Locator pools created.
History cache pools created.
Entities: reserved=98, maximum=4294967295
Domain pools created.
RTPS Initialised.
IP: scope = link..global
IP interfaces:
    eth0     : 192.168.0.3
RTPS over IPv4 Initialised.
QoS pools initialized.
DDS: core thread running.
DDS: core thread created.
DDS: DynamicType_register(ParticipantMessageData): 5 blocks, 264 bytes
Discovery initialized.
DCPS Initialised.
UDP: adding 192.168.0.3:7589 on [9]
UDP: adding 239.255.0.1:7401 on [10]
UDP: setsockopt (SO_REUSEADDR) 239.255.0.1:7401
UDP: ipmsfilter join group 239.255.0.1
RTPS: starting Discovery protocols.
UDP: adding 192.168.0.3:7588 on [11]
UDP: adding 239.255.0.1:7400 on [12]
UDP: setsockopt (SO_REUSEADDR) 239.255.0.1:7400
UDP: ipmsfilter join group 239.255.0.1
SPDP: registering Participant key.
SPDP: Send Participant data.
mm_malloc: Allocation failed, size 16
mm_malloc: Allocation failed, size 32
Can't create chat message type!

Conclusions:

Having tested the RAM requirements this way allows to see how the code behaves with different sizes configured. The memory configuration for the microcontroller is defined in the linker script:

MEMORY
{
    flash (rx) : ORIGIN = 0x08000000, LENGTH = 1024K
    sram (rwx) : ORIGIN = 0x20000000, LENGTH = 128K
}

With this in mind it seems like:

  • The 64-Kbyte of CCM (core coupled memory) are not useful (or at least I have not been able to make them work)
  • A minimum of 32KB of addicional SRAM is needed for a simple pub/sub program
  • If more than 100 KB of SRAM is available i can see a performance improve

@jvoe
Copy link

jvoe commented Nov 18, 2014

@vmayoral Can you show the output of the 'spool' command?

@vmayoral
Copy link
Author

@jvoe sure, here it is:

?spool

  Name          PSize  BSize  Rsrvd    Max MPUse CPUse MXUse CXUse Alloc  NoMem
  ----          -----  -----  -----    --- ----- ----- ----- ----- -----  -----
List:
  SLIST             0     80      0      *     0     0     0     0     0      0
  SL0               0     16      0      *     0     0    35    35    35      0
  SL1               0     24      0      *     0     0     9     9     9      0
  SL2               0     32      0      *     0     0     0     0     0      0
  SL3               0     40      0      *     0     0     0     0     0      0
  SL4               0     48      0      *     0     0     0     0     0      0
  SL5               0     56      0      *     0     0     0     0     0      0
  SL6               0     64      0      *     0     0     0     0     0      0
  SL7               0     72      0      *     0     0     0     0     0      0
String:
  STRING            0     16      0      *     0     0    45    39    47      0
  STR_REF           0      8      0      *     0     0    45    39    47      0
Timer:
  TIMER             0     24      0      *     0     0     6     1    10      0
Buffers:
  DATA_BUF(0)       0   8208      0      *     0     0     0     0     0      0
  DATA_BUF(1)       0   4112      0      *     0     0     0     0     0      0
  DATA_BUF(2)       0   2064      0      *     0     0     0     0     0      0
  DATA_BUF(3)       0   1040      0      *     0     0     0     0     0      0
  DATA_BUF(4)       0    528      0      *     0     0     0     0     0      0
  DATA_BUF(5)       0    272      0      *     0     0     0     0     0      0
  DATA_BUF(6)       0    144      0      *     0     0     0     0     0      0
  DATA_BUF(7)       0     80      0      *     0     0     0     0     0      0
Locators:
  LOCREF            0      8      0      *     0     0    53    52    74      0
  LOCATOR           0     40      0      *     0     0     8     8     8      0
QoS:
  QOS_REF           0      8      0      *     0     0     3     3     3      0
  QOS_DATA          0     88      0      *     0     0     3     3     3      0
Cache:
  HIST_CACHE        0    152      0      *     0     0    10    10    10      0
  CHANGE            0     48      0      *     0     0     9     7   382      0
  INSTANCE          0    128      0      *     0     0     6     5    11      0
  CCREF             0     16      0      *     0     0    10    10   612      0
  CREF              0      8      0      *     0     0     1     1     1      0
  CWAIT             0     32      0      *     0     0     0     0     0      0
  CXFER             0     48      0      *     0     0     0     0     0      0
  XFLIST            0     32      0      *     0     0     0     0     0      0
  FILTER            0     64      0      *     0     0     0     0     0      0
  FINST             0     40      0      *     0     0     0     0     0      0
Domain:
  DOMAIN            0    904      0      5     0     0     1     1     1      0
  DPARTICIPANT      0    336      0      *     0     0     1     1     1      0
  TYPE              0     24      0      *     0     0     5     5     5      0
  TOPIC             0     72      0      *     0     0     9     9     9      0
  FILTER_TOPIC      0    120      0      *     0     0     0     0     0      0
  PUBLISHER         0    200      0      *     0     0     2     2     2      0
  SUBSCRIBER        0    192      0      *     0     0     2     2     2      0
  WRITER            0    144      0      *     0     0     5     5     5      0
  READER            0    232      0      *     0     0     5     5     5      0
  DWRITER           0     40      0      *     0     0     4     4     4      0
  DREADER           0     56      0      *     0     0     4     4     4      0
  GUARD             0     40      0      *     0     0     0     0     0      0
  PREFIX            0     24      0      *     0     0     1     0     4      0
DDS:
  NOTIFICATION      0     16      0      *     0     0     1     0     6      0
  WS_DEFERED        0     16      0      *     0     0     0     0     0      0
  CFG_UPDATE        0     16      0      *     0     0     0     0     0      0
DCPS:
  SAMPLE_INFO       0     56      0      *     0     0     0     0     0      0
  WAITSET           0     32      0      *     0     0     0     0     0      0
  STATUS_COND       0     32      0      *     0     0     0     0     0      0
  READ_COND         0     40      0      *     0     0     0     0     0      0
  QUERY_COND        0     80      0      *     0     0     0     0     0      0
  GUARD_COND        0     24      0      *     0     0     0     0     0      0
  TOPIC_WAIT        0     24      0      *     0     0     0     0     0      0
RTPS:
  READER            0     40      0      *     0     0     5     5     5      0
  WRITER            0     48      0      *     0     0     5     5     5      0
  REM_READER        0    144      0      *     0     0     5     5     5      0
  REM_WRITER        0    128      0      *     0     0     4     4     4      0
  CCREF             0     32      0      *     0     0     3     2    78      0
  MSG_BUF           0    160      0      *     0     0     4     0   176      0
  MSG_ELEM_BUF      0    120      0      *     0     0     4     0   180      0
  MSG_REF           0      8      0      *     0     0     0     0     0      0
IP:
  IP_CX             0    144      0   1025     0     0     5     5     5      0
XTYPES:
  DYNTYPE           0     16      0      *     0     0     7     1    11      0
  DYNDREF           0     16      0      *     0     0     1     1   150      0

Pool/max/xmax/used/xused memory = 8192/8192/14752/8192/12968 bytes (1919 mallocs) (280%/258%)
Dynamically allocated: 2267 bytes.
malloc statistics: 639 blocks, 38210 bytes, 0 failures.
realloc statistics: 4 blocks, 136 bytes, 0 failures.
free statistics: 613 blocks, 36079 bytes.
Dynamic pool block stats: <=64K - max/used/msize/size: 5/3/496/496
                           >64K - max/used           : 0/0
Total heap memory: 22944 bytes.

@vmayoral
Copy link
Author

@jvoe I'm trying to test interoperability between other DDS implementations and Tinq starting from an IDL message.

I've generated the corresponding C code from the IDL message using the idl app however the code generated does not compile (procedure). Do you recall having had problems with this matter? I'll try replicating the code of the chat and build it from there.

Also, we are considering creating a middleware for Tinq so that ROS2 can sit on top (something similar to https://github.com/ros2/ros_middleware_opensplice). Although Tinq is highly configurable with the compiling definitions, would it be ok if we create a shared library so that ROS2-Tinq applications can compile against?

@jvoe
Copy link

jvoe commented Nov 19, 2014

@vmayoral The 'spool' output shows a normal memory usage pattern - nothing abnormal to see there. I'm not sure where the huge amount of data comes from.

The 'idl' tool was written as a first attempt to convert pre- X-types IDL data specifications to TSM C-data structs which are an intermediate format used for typecode generation. It sort of works, except that it has a few restrictions:

  • It doesn't accept the later IDL extensions that were defined in the X-Types spec.
  • The TSMs that are generated lack a field that was added in a later stage of development.
  • It's a bit buggy when modules are used.

I often used it myself with a bit of manual 'tweaking' so it is useful.

The tool was never finalized/maintained properly since the decision was taken for Qeo to use QDM data definitions (QDM=Qeo Data Model, which is an XML-data representation, based on the DDS XML-type definition from the X-types spec). The QDM supports a subset of the the full X-types specification, whereas using TSMs and/or directly calling X-types functions allow the (almost) complete X-types functionality to be used.

The chat app uses extensibility = mutable types, which cannot be handled by the idl tool. It would not be difficult to change it to use 'normal', i.e. pre- X-types type definitions, which can be handled by the idl tool. The reason for this is again a Qeo decision to use exclusively mutable types in order to better handle possible future type changes.

As to creating a shared library for ROS2, I don't think the BSD license is against that ... The code is free to use, except for the Qeo copyright. But I'm not a lawyer, so don't take my word for it. ;-)

@vmayoral
Copy link
Author

@jvoe thanks for the input. I've been trying to actually make an imu_publisher based on the chat implementation.

I tried removing the MUTABLE specific code to come back to x-types (which is ideal for us to integrate with ROS2) however the simple imu_publisher code seems to have issues after the removal of the MUTABLE code. When, launched:

DDS_DataWriter_register_instance: invalid parameters!
IMU accel message: x=3.000000, y=0.000000 and z=0
DDS_DataWriter_register_instance: invalid parameters!
IMU accel message: x=3.000000, y=0.000000 and z=0
DDS_DataWriter_register_instance: invalid parameters!
IMU accel message: x=3.000000, y=0.000000 and z=0
...

Which fails when fetching the data key. Is this something expected from x-types or is it related to " extensibility = mutable types"?

@jvoe
Copy link

jvoe commented Nov 20, 2014

@vmayoral Does your new type contain key fields? If not, you shouldn't call DDS_DataWriter_register_instance(). Which fields are key fields is specified with the "@key" annotation in X-types.

@vmayoral
Copy link
Author

Thanks @jvoe that helped progressing.

I moved forward with the Tinq (Desktop) <-> OpenSplice (Desktop) interoperability tests and I'm struggling with this matter. It seems that Tinq and OpenSplice can indeed talk but the messages are not passed appropriately. Please refer to ros2/ros2_embedded_nuttx#25 (comment).

I have not been able to make it work with RTI's Connext neither (actually not even discovery).

Could you please provide input in this matter? Has Tinq been tested against other DDS implementations?

@gregory-nutt
Copy link

@jvoe I don't know if you are still looking at the UDP routing logic but if so, I want to give you a heads up that it just got a significant change today. I am working to support multiple network interfaces and that section of code needed some heavy touches to (1) handle the same port numbers on different interfaces and also (2) to properly route incoming UDP packets. The basic change is the with a single network interface, you can mostly ignore the bound IP address of the UDP socket and do everything with the UDP port. But not so with multiple network interfaces.

If you are still looking at this, your inputs and thoughts would be appreciated.

Greg

@jvoe
Copy link

jvoe commented Nov 23, 2014

@NuttX I did have a look at the UDP and socket code at the time when we discussed it. When @vmayoral managed to get the receive sockets working I stopped changing the code, figuring it wasn't that useful anymore. There were only a few changes done, tbh, mainly data, i.e. I just added a queue structure to the UDP socket and some bookkeeping variables, and added init and cleanup code for those. So nothing of much use to you, I guess.

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

No branches or pull requests

8 participants