add native platform #98
Conversation
…enable cmake option. phy: some defensive programming changes to prevent possible divide by zero and stack overrun. serial_interface: if PLATFORM_MODEM_INTERFACE_UART is not defined then don't try to register a serial interface. alp_layer: initialise with a default D7 interface. alp_layer: add support for application layer to accept write file operations in the same manner as read file operations. Useful if we want to bypass fs completely. log: added new stack logging options to give improved trace read out. log: added timestamps to logging output. d7ap: these structs are used for constructing payloads in the stack and rely on byte alignment of members. platforms/native: now supports emulated uart, timer, radio and buttons. hwradio: extended API for platform/native implementation only. console: new platform/native console option added. apps/sensor_pull: bug fix as fs layer is not initialised before initialising the stack. platforms/B_L072Z_LRWAN1: add support for USART4 which is excellent for TX pini only debug logging as an alt. to the JLink output. platforms/B_L072Z_LRWAN1: add support for including GPS chip into platform build. chip/neo_m8n_gps: new chip driver support (using UART) for the M8N GPS receiver. apps/gateway_gps: new variant GPS gateway for testing GPS enabled sensor. apps/sensor_gps: new GPS enabled sensor application.
|
Thanks for this extensive work! I will need some time to review this but will come back to you |
There was a problem hiding this comment.
Thanks again for this very useful contribution. I added some review comments inline.
More generally: I would suggest to remove the extra apps and GPS driver introduced here. I think it is better to not add examples and drivers for a specific use case in this repo, to keep the code base smaller and better maintainable. Most users are not interested in a specific GPS chip for instance. It can be added to an external repo if needed, as described here http://mosaic-lopow.github.io/dash7-ap-open-source-stack/docs/out-of-tree/
| @@ -49,7 +71,7 @@ __LINK_C void log_print_string(char* format, ...) | |||
| { | |||
| va_list args; | |||
| va_start(args, format); | |||
| printf("\n\r[%03d] ", NG(counter)++); | |||
| printf("\n\r[%u] [%03d] ", timer_get_counter_value(), NG(counter)++ ); | |||
There was a problem hiding this comment.
This will make logging more expensive when running on an MCU. I suggest to remove this or make it configurable?
There was a problem hiding this comment.
Do you think the overhead is that large? Still it does no harm to make it a compile-time option.
| @@ -58,14 +80,14 @@ __LINK_C void log_print_stack_string(log_stack_layer_t type, char* format, ...) | |||
| { | |||
| va_list args; | |||
| va_start(args, format); | |||
| printf("\n\r[%03d] ", NG(counter)++); | |||
| printf("\n\r[%u] [%03d] [%s] ", timer_get_counter_value(), NG(counter)++, stack_layer_str[type]); | |||
| vprintf(format, args); | ||
| va_end(args); | ||
| } | ||
|
|
||
| __LINK_C void log_print_data(uint8_t* message, uint32_t length) | ||
| { | ||
| printf("\n\r[%03d]", NG(counter)++); | ||
| printf("\n\r[%u] [%03d]", timer_get_counter_value(), NG(counter)++); |
| @@ -79,7 +101,7 @@ void log_print_error_string(char* format,...) | |||
| { | |||
| va_list args; | |||
| va_start(args, format); | |||
| printf("\n\r%s[%03d]%s ", RTT_CTRL_BG_BRIGHT_RED, NG(counter)++, RTT_CTRL_RESET); | |||
| printf("\n\r%s[%u] [%03d]%s ", RTT_CTRL_BG_BRIGHT_RED, timer_get_counter_value(), NG(counter)++, RTT_CTRL_RESET); | |||
| @@ -33,9 +33,31 @@ | |||
| #include "framework_defs.h" | |||
| #include "hwsystem.h" | |||
| #include "SEGGER_RTT.h" | |||
| #include "timer.h" | |||
There was a problem hiding this comment.
Can be removed when not used (see comment below)
| @@ -0,0 +1,43 @@ | |||
| # Overview | |||
There was a problem hiding this comment.
this should be moved to the documentation site (http://mosaic-lopow.github.io/dash7-ap-open-source-stack/docs/home/) by moving this to docs/_docs/platform-native.md and linking to this from docs/_docs/hardware.md and docs/_data/docs.yaml
|
|
||
| PYTHONPATH=`pwd` python examples/query_nodes.py -d /tmp/S1 | ||
|
|
||
| # Related repositories |
There was a problem hiding this comment.
copy paste leftover?
| @@ -457,6 +457,10 @@ void hw_radio_set_tx_power(int8_t eirp); | |||
|
|
|||
| void hw_radio_set_rx_timeout(uint32_t timeout); | |||
|
|
|||
| void hwradio_set_addr(int); | |||
There was a problem hiding this comment.
All 3 of the API functions seem rather specific to native, hence I would not add them to the public API but keep it internal?
There was a problem hiding this comment.
Ok. Is there any convention I can follow for the private API calls?
| // a single local machine, one can use the PID to set the address since PID | ||
| // is unique on a single machine. | ||
|
|
||
| struct radio_packet_header |
There was a problem hiding this comment.
I propose to add __attribute__((__packed__)) here, this makes it easier to distinguish the fields when looking at network traces in wireshark for instnace
| @@ -68,7 +68,9 @@ typedef enum | |||
| AES_CCM_32 = 0x07, /* data confidentiality and authenticity*/ | |||
| } nls_method_t; | |||
|
|
|||
| typedef struct { | |||
| #pragma pack(1) | |||
There was a problem hiding this comment.
why is this necessary?
There was a problem hiding this comment.
Indeed, I am not sure why this should be any different to __attribute__((__packed__)) but the compiler generates different code which breaks the packet construction unless I use #pragma pack(1) when compiling natively.
glennergeerts
changed the title
|
|
||
| Run the following command to build using the assigned device /tmp/S0 from socat above: | ||
|
|
||
| cmake <stack_directory> -DCMAKE_TOOLCHAIN_FILE=<stack_directory>/cmake/toolchains/gcc.cmake -DAPP_GATEWAY=y -DAPP_SENSOR_PUSH=y -DAPP_SENSOR_PULL=y -DPLATFORM_UART_DEV0=/tmp/S0 |
There was a problem hiding this comment.
I think this is missing a -DPLATFORM=NATIVE parameter.
|
|
||
| socat pty,link=/tmp/S0,raw,echo=0 pty,link=/tmp/S1,raw,echo=0 & | ||
|
|
||
| Run the following command to build using the assigned device /tmp/S0 from socat above: |
There was a problem hiding this comment.
Right now the fd to use for the UART is a build time parameter, which means that on one host PC it is only possible to run one node which uses UART right? Unless you have multiple build directories but this is cumbersome. I was wondering if it would not be easier and more flexible to pass the filename to use as a parameter to the binary (eg ./modem -d /tmp/S1)? This would simplify the build a bit and allow to start multiple nodes using different serial ports easily.
This is just a thought, and is not something which is blocking this PR. It can be done afterwards just as well if we decide to do so. WDYT?
|
That's a great idea concerning the UART parameter. I'll make that change
along with the other recommendations.
…On Thu, 19 Nov 2020, 08:15 glennergeerts, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In stack/framework/hal/platforms/NATIVE/README.md
<#98 (comment)>
:
> +# Why is this useful?
+
+- Access to physical hardware is not always possible
+- Allow application and stack to be tested independently of hardware
+- Full stack debug trace can be CPU intensive on small embedded CPUs
+- More rapidly try out stack or application ideas
+
+# Quick start
+
+Launch virtual serial ports using socat as follows:
+
+socat pty,link=/tmp/S0,raw,echo=0 pty,link=/tmp/S1,raw,echo=0 &
+
+Run the following command to build using the assigned device /tmp/S0 from socat above:
+
+cmake <stack_directory> -DCMAKE_TOOLCHAIN_FILE=<stack_directory>/cmake/toolchains/gcc.cmake -DAPP_GATEWAY=y -DAPP_SENSOR_PUSH=y -DAPP_SENSOR_PULL=y -DPLATFORM_UART_DEV0=/tmp/S0
I think this is missing a -DPLATFORM=NATIVE parameter.
------------------------------
In stack/framework/hal/platforms/NATIVE/README.md
<#98 (comment)>
:
> + on your Linux machine via the emulated UART layer
+
+# Why is this useful?
+
+- Access to physical hardware is not always possible
+- Allow application and stack to be tested independently of hardware
+- Full stack debug trace can be CPU intensive on small embedded CPUs
+- More rapidly try out stack or application ideas
+
+# Quick start
+
+Launch virtual serial ports using socat as follows:
+
+socat pty,link=/tmp/S0,raw,echo=0 pty,link=/tmp/S1,raw,echo=0 &
+
+Run the following command to build using the assigned device /tmp/S0 from socat above:
Right now the fd to use for the UART is a build time parameter, which
means that on one host PC it is only possible to run one node which uses
UART right? Unless you have multiple build directories but this is
cumbersome. I was wondering if it would not be easier and more flexible to
pass the filename to use as a parameter to the binary (eg ./modem -d
/tmp/S1)? This would simplify the build a bit and allow to start multiple
nodes using different serial ports easily.
This is just a thought, and is not something which is blocking this PR. It
can be done afterwards just as well if we decide to do so. WDYT?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#98 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABTRSTZBBC6UFVZJCX4WMUTSQTH2DANCNFSM4TH3BB3A>
.
|
|
Hi, are you still working on this PR? Do you need any help from us? |
|
Hi Glenn,
Been a bit busy. Will try to get onto it this week during an evening!
Liam.
…On Mon, 1 Feb 2021 at 07:57, glennergeerts ***@***.***> wrote:
Hi, are you still working on this PR? Do you need any help from us?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#98 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABTRST3DNOK45T4SHMXAAGDS4ZNHNANCNFSM4TH3BB3A>
.
|
|
Sure, I was just curious ;) |
Resolve "Split alp_layer and D7" Closes Sub-IoT#98 See merge request aloxy/oss-7!65
This PR provides some improvements to the NATIVE build support. Refer to the README.md file under stack/framework/hal/platforms/NATIVE for information on how to build and run some applications natively on Linux.
The native build supports:
There are some other changes included in this PR relating to some minor bugs I've encountered in the stack during the development of this PR. I've also extended the alp_layer to support application completion of write ALP commands in much the same manner as read ALP commands, as it led to nicer code not having to do everything explicitly through the file system layer.
This PR also has some new applications and chip support for the U-blox M8N GPS chip which will work both with the native build (if you have one of their "click" eval. boards attached to your computer via a serial port or FTDI cable) and the B_L072Z_LRWAN1 platform.