Module to Read and Broadcast Decawave Serial Communication within Paparazzi #2154

coppolam · 2017-10-31T16:30:54Z

The module works with Decawave DWM1000 modules on Arduino, allowing multiple modules to communicate velocity and height from paparazzi with each-other, on top of measuring inter-antenna range. The link to the library and Arduino build file is provided in the module description. An ABI message with the received information is then broadcasted.

gautierhattenberger · 2017-11-01T10:38:20Z

How is this module different from http://docs.paparazziuav.org/latest/module__dw1000_arduino.html ? Can we merge the two functionality if not exactly the same ?

coppolam · 2017-11-01T11:55:01Z

The module differs in that it is made for anchorless networks where UWB does not function as a trilateriation technology, but as a ranging/communication technology between MAVs. The module enables communication paparazzi on-board variables between MAVs (in this case VX, VY, Z). The agents need to communicate these values in order to achieve anchorless relative localization. From my understanding, this is a different functionality than dw1000_arduino and hence could be kept separate.

kirkscheper

Looks like you added opencv_bebop and pprzlink changes

kirkscheper · 2017-11-01T12:12:59Z

sw/airborne/modules/decawave/decawave_serial_communication.c

+};
+
+static uint8_t _bytesRecvd = 0;
+static uint8_t _dataRecvCount = 0;


Why is this global? It is only used in the decode function.

kirkscheper · 2017-11-01T12:14:06Z

sw/airborne/modules/decawave/decawave_serial_communication.c

+
+static uint8_t _tempBuffer[UWB_SERIAL_COMM_MAX_MESSAGE];
+static uint8_t _tempBuffer2[UWB_SERIAL_COMM_MAX_MESSAGE];
+static uint8_t _recvBuffer[UWB_SERIAL_COMM_FLOAT_SIZE];


kirkscheper · 2017-11-01T12:15:11Z

sw/airborne/modules/decawave/decawave_serial_communication.c

+ * the payload values 253, 254, and 255 are encoded as 2 bytes, respectively 253 0, 253 1, and 253 2.
+ */
+static void encodeHighBytes(uint8_t* sendData, uint8_t msgSize){
+	_dataSendCount = msgSize;


why is this used at all, why not just loop over msgSize?

kirkscheper · 2017-11-01T12:17:02Z

sw/airborne/modules/decawave/decawave_serial_communication.c

+static uint8_t _dataRecvCount = 0;
+
+static uint8_t _tempBuffer[UWB_SERIAL_COMM_MAX_MESSAGE];
+static uint8_t _tempBuffer2[UWB_SERIAL_COMM_MAX_MESSAGE];


Can you use more descriptive naming here. _tempBuffer seems to be a receive buffer and 2 seems to be a send buffer so e.g. receive_data_buffer send_data_buffer

kirkscheper · 2017-11-01T12:20:35Z

sw/airborne/modules/decawave/decawave_serial_communication.c

+    _dataRecvCount++;
+  }
+  if(_dataRecvCount==UWB_SERIAL_COMM_FLOAT_SIZE){
+    memcpy(&tempfloat,&_recvBuffer,UWB_SERIAL_COMM_FLOAT_SIZE);


This doesn't look right.

kirkscheper · 2017-11-01T12:22:40Z

sw/airborne/modules/decawave/decawave_serial_communication.h

+#define UWB_SERIAL_COMM_VX 0
+#define UWB_SERIAL_COMM_VY 1
+#define UWB_SERIAL_COMM_Z 2
+#define UWB_SERIAL_COMM_RANGE 3


Why are these defines in the header? Who else would want to use these?

gautierhattenberger · 2017-11-01T13:34:16Z

conf/abi.xml

+    <message name="UWB_COMMUNICATION" id="19">
+      <field name="id" type="uint8_t"/>
+      <field name="range" type="float" unit="m"/>
+      <field name="vx" type="float" unit="m/s^2"/>


Is this acceleration or speed ? Unit and name are not matching

gautierhattenberger · 2017-11-01T13:35:32Z

conf/modules/decawave_serial_communication.xml

+  <event fun="decawave_serial_communication_event()"/>
+  <makefile target="ap">
+    <file name="decawave_serial_communication.c" />
+    <configure name="SERIAL_UART" default="uart2" case="upper|lower" />


SERIAL_* is a bit too generic, it should be at least prefixed by something in relation with the module name

gautierhattenberger · 2017-11-01T13:38:24Z

sw/airborne/modules/decawave/decawave_serial_communication.c

+#define SERIAL_PORT   (&((SERIAL_UART).device))
+struct link_device *xdev = SERIAL_PORT;
+
+#define SerialGetch() SERIAL_PORT ->get_byte(SERIAL_PORT->periph)


Not sure these macros are a big help. Sometimes you use SERIAL_PORT, sometimes xdev (which is not a very explicit name). Would be better to be consistent.

gautierhattenberger · 2017-11-01T13:40:03Z

sw/airborne/modules/decawave/decawave_serial_communication.c

+
+static struct nodeState _states[UWB_SERIAL_COMM_DIST_NUM_NODES];
+float range_float = 0.0;
+


In general, you are using here a lot of global variables that could be gathered in a single structure with the buffers, status flags, indexes, uart device pointer, ...

gautierhattenberger · 2017-11-01T13:41:33Z

sw/airborne/modules/decawave/decawave_serial_communication.c

+    }
+    _dataRecvCount++;
+  }
+  if(_dataRecvCount==UWB_SERIAL_COMM_FLOAT_SIZE){


when you're done, don't forget to run the fix_code_style.sh script on your files

gautierhattenberger · 2017-11-01T21:10:20Z

sw/airborne/modules/decawave/decawave_anchorless_communication.c

+#include "subsystems/radio_control.h"
+#include "state.h"
+#include "pprzlink/messages.h"
+#include "subsystems/datalink/downlink.h"


is those includes (downlink.h, messages.h) really needed ?

gautierhattenberger · 2017-11-01T21:39:22Z

Mostly okay. It seems that your files don't have a endline character at the end.
Also, the default baudrate is only 9600 bauds. It looks a bit slow for an application that potentially require a high update rate. Don't you think it could be higher ?

StevenH2812 · 2017-11-02T12:17:26Z

@gautierhattenberger It has been tested and working with 9600 baud rate. The lower baud rate ensures that the decawave side will function also with slower microprocessors. For the basic functionality of the module 9600 baud rate is enough since the communication is kept minimal. (6 byte messages, 4 messages back and forth, 20-30 Hz update frequency = 3840-5760 bit/s). If you have a faster microprocessor then by all means you should increase it to ensure you always have the latest data available, but people can always do so if they do decide to use this module. I noticed with an 8 MHz Atmega328 that 57600 was the limit baud rate, higher baud rates started giving erroneous communication.

That being said, the default can be set higher to say 19200 as well. I think whoever uses this module should tune this value to whatever works well with the microprocessor used on the decawave side anyways, so it's quite arbitrary.

…arazzi (#2154) * added decawave_serial_communication module files * added reference to arduino library in module comments * added abi message * fixing abi send * added abi * compiles * tested on bebop2 drones with decawave modules * improved module description * cleanup * fixed code-style * pprzlink same as master * updated subrepos opencv_bebop * temp * temp * temp * addressed define and global variable comments * Addressed comments on define macros and abi.xml * making functions more general * fix up * tested! * removing last global variables * renamed temp variables * must have messed up the message decoding/sending * successfully tested * clean-up * changed variable names as necessary * float size define removed * last changes + tests * removed test module * removed unnecessary includes and added endline characters

…arazzi (paparazzi#2154) * added decawave_serial_communication module files * added reference to arduino library in module comments * added abi message * fixing abi send * added abi * compiles * tested on bebop2 drones with decawave modules * improved module description * cleanup * fixed code-style * pprzlink same as master * updated subrepos opencv_bebop * temp * temp * temp * addressed define and global variable comments * Addressed comments on define macros and abi.xml * making functions more general * fix up * tested! * removing last global variables * renamed temp variables * must have messed up the message decoding/sending * successfully tested * clean-up * changed variable names as necessary * float size define removed * last changes + tests * removed test module * removed unnecessary includes and added endline characters

gautierhattenberger self-requested a review November 1, 2017 10:38

gautierhattenberger added Airborne Duplicate Add the label if the issue looks exactly like on reported already Module labels Nov 1, 2017

kirkscheper reviewed Nov 1, 2017

View reviewed changes

gautierhattenberger requested changes Nov 1, 2017

View reviewed changes

coppolam added 15 commits November 1, 2017 14:47

added decawave_serial_communication module files

6e698fb

added reference to arduino library in module comments

c3b8a45

added abi message

75d8000

fixing abi send

7342134

added abi

75fde55

compiles

eaf1e4e

tested on bebop2 drones with decawave modules

3c919ee

improved module description

631e23b

cleanup

857ef9f

fixed code-style

a71792c

pprzlink same as master

408e50f

updated subrepos opencv_bebop

b06fcca

temp

9bf44d4

temp

42fe08a

temp

951d9ab

coppolam force-pushed the pullrequest_decawave_serial_communication branch from 68dcd24 to 951d9ab Compare November 1, 2017 14:49

coppolam added 6 commits November 1, 2017 16:09

addressed define and global variable comments

6e63a08

Addressed comments on define macros and abi.xml

2e65b39

making functions more general

8d78f71

fix up

517b885

tested!

2c3dad8

removing last global variables

adc6226

coppolam added 8 commits November 1, 2017 19:04

renamed temp variables

d5d83c9

must have messed up the message decoding/sending

b1d148a

successfully tested

dcb7f04

clean-up

9630079

changed variable names as necessary

f9ebd87

float size define removed

ed05770

last changes + tests

77c6973

removed test module

5fda5f1

gautierhattenberger reviewed Nov 1, 2017

View reviewed changes

removed unnecessary includes and added endline characters

f5721b9

kirkscheper approved these changes Nov 2, 2017

View reviewed changes

gautierhattenberger approved these changes Nov 2, 2017

View reviewed changes

gautierhattenberger merged commit b5a4f89 into paparazzi:master Nov 2, 2017

coppolam deleted the pullrequest_decawave_serial_communication branch November 3, 2017 13:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Module to Read and Broadcast Decawave Serial Communication within Paparazzi #2154

Module to Read and Broadcast Decawave Serial Communication within Paparazzi #2154

coppolam commented Oct 31, 2017

gautierhattenberger commented Nov 1, 2017

coppolam commented Nov 1, 2017

kirkscheper left a comment

kirkscheper Nov 1, 2017

kirkscheper Nov 1, 2017

kirkscheper Nov 1, 2017

kirkscheper Nov 1, 2017

kirkscheper Nov 1, 2017

kirkscheper Nov 1, 2017

gautierhattenberger Nov 1, 2017

gautierhattenberger Nov 1, 2017

gautierhattenberger Nov 1, 2017

gautierhattenberger Nov 1, 2017

gautierhattenberger Nov 1, 2017

gautierhattenberger Nov 1, 2017

gautierhattenberger commented Nov 1, 2017

StevenH2812 commented Nov 2, 2017


		static struct nodeState _states[UWB_SERIAL_COMM_DIST_NUM_NODES];
		float range_float = 0.0;

Module to Read and Broadcast Decawave Serial Communication within Paparazzi #2154

Module to Read and Broadcast Decawave Serial Communication within Paparazzi #2154

Conversation

coppolam commented Oct 31, 2017

gautierhattenberger commented Nov 1, 2017

coppolam commented Nov 1, 2017

kirkscheper left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gautierhattenberger commented Nov 1, 2017

StevenH2812 commented Nov 2, 2017