Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

paho: use GNRC instead of lwip #18982

Merged
merged 1 commit into from
Dec 1, 2022
Merged

paho: use GNRC instead of lwip #18982

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 4 additions & 16 deletions examples/paho-mqtt/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,24 +34,12 @@ USEMODULE += netdev_default
USEPKG += paho-mqtt

# paho-mqtt depends on TCP support, choose which stacks you want
GNRC_IPV6 ?= 1
LWIP_IPV4 ?= 0
Copy link
Contributor

@aabadie aabadie Nov 27, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switching to GNRC will drop support for IPv4 from this example. I think the use of LwIP stack was not only motivated by the missing TCP support in GNRC but also by the missing IPv4 support.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, the main motivation for this PR was that the LWIP version in the IoT-Lab on IPv6 doesn't work for some reason. (The TCP connection is not established.) Since I didn't want to debug LWIP, I tried with GNRC instead.

In the end, the question remains: what is the main use case we want to support here. My idea is that all networking examples use GNRC whenever possible. Of course, that limits the use case to IPv6 (and 6LoWPAN) only. The other option would be to use LWIP for all networking examples wherever possible to allow for both, IPv4 and IPv6.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But with the sock API we can support both stacks without any changes to the code.
So the Makefile only needs to select different modules, depending on the desired stack.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, first of all that would require that this application actually works with both stacks which I couldn't verify (LWIP with 6lbr doesn't seem to work). Second, if we go with this reasoning we should update all networking applications which use sock.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use LWIP without 6lbr, it also supports non-6lo interfaces.

It's not unreasonable to try to build with both stacks to ensure they work the same. The changes to the Makefile are really small.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use LWIP without 6lbr, it also supports non-6lo interfaces.

I would prefer an example that works in all scenarios. Having an example application that is known to be broken in a certain configuration may get frustrating for a user.

It's not unreasonable to try to build with both stacks to ensure they work the same. The changes to the Makefile are really small.

I'm not per se against this, but what would be the rationale to enable both stacks for this application, but, for instance, not for the MQTT-SN examples?

LWIP_IPV6 ?= 1
LWIP_IPV6 ?= 0

ifneq (0,$(LWIP_IPV4))
USEMODULE += ipv4_addr
USEMODULE += lwip_arp
USEMODULE += lwip_ipv4
USEMODULE += lwip_dhcp_auto
CFLAGS += -DETHARP_SUPPORT_STATIC_ENTRIES=1
endif

ifneq (0,$(LWIP_IPV6))
USEMODULE += ipv6_addr
USEMODULE += lwip_ipv6_autoconfig
endif

USEMODULE += lwip_netdev
USEMODULE += lwip
include Makefile.lwip
include Makefile.gnrc

USEMODULE += sock_async_event
USEMODULE += sock_ip
Expand Down
2 changes: 2 additions & 0 deletions examples/paho-mqtt/Makefile.ci
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
BOARD_INSUFFICIENT_MEMORY := \
airfy-beacon \
blackpill \
blackpill-128kib \
bluepill \
bluepill-128kib \
bluepill-stm32f030c8 \
calliope-mini \
hifive1 \
Expand Down
16 changes: 16 additions & 0 deletions examples/paho-mqtt/Makefile.gnrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
ifneq (0,$(GNRC_IPV6))
ifneq (0,$(USE_LWIP))
$(error No valid choice: Select either LWIP or GNRC)
endif
USEMODULE += auto_init_gnrc_netif
# Activate ICMPv6 error messages
USEMODULE += gnrc_icmpv6_error
# Specify the mandatory networking modules for IPv6
USEMODULE += gnrc_ipv6_default
# Additional networking modules that can be dropped if not needed
USEMODULE += gnrc_icmpv6_echo
else
ifeq (0,$(USE_LWIP))
$(error No network stack selected. Please choose either GNRC or LWIP)
endif
endif
25 changes: 25 additions & 0 deletions examples/paho-mqtt/Makefile.lwip
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
USE_LWIP := 0
ifneq (0,$(LWIP_IPV4))
USE_LWIP := 1
endif
ifneq (0,$(LWIP_IPV6))
USE_LWIP := 1
endif

ifneq (0,$(LWIP_IPV4))
USEMODULE += ipv4_addr
USEMODULE += lwip_arp
USEMODULE += lwip_ipv4
USEMODULE += lwip_dhcp_auto
CFLAGS += -DETHARP_SUPPORT_STATIC_ENTRIES=1
endif

ifneq (0,$(LWIP_IPV6))
USEMODULE += ipv6_addr
USEMODULE += lwip_ipv6_autoconfig
endif

ifneq (0,$(USE_LWIP))
USEMODULE += lwip_netdev
USEMODULE += lwip
endif
6 changes: 6 additions & 0 deletions examples/paho-mqtt/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@
#include "paho_mqtt.h"
#include "MQTTClient.h"

#define MAIN_QUEUE_SIZE (8)
static msg_t _main_msg_queue[MAIN_QUEUE_SIZE];

#define BUF_SIZE 1024
#define MQTT_VERSION_v311 4 /* MQTT v3.1.1 version is 4 */
#define COMMAND_TIMEOUT_MS 4000
Expand Down Expand Up @@ -293,6 +296,9 @@ static unsigned char readbuf[BUF_SIZE];

int main(void)
{
if (IS_USED(MODULE_GNRC_ICMPV6_ECHO)) {
msg_init_queue(_main_msg_queue, MAIN_QUEUE_SIZE);
}
#ifdef MODULE_LWIP
/* let LWIP initialize */
ztimer_sleep(ZTIMER_MSEC, 1 * MS_PER_SEC);
Expand Down
4 changes: 1 addition & 3 deletions pkg/paho-mqtt/contrib/riot_iface.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,9 @@
#define TSRB_MAX_SIZE (1024)
#endif

#ifdef MODULE_LWIP
static uint8_t buffer[TSRB_MAX_SIZE];
static uint8_t _temp_buf[TSRB_MAX_SIZE];
static tsrb_t tsrb_lwip_tcp;
#endif

#ifndef PAHO_MQTT_YIELD_MS
#define PAHO_MQTT_YIELD_MS (10)
Expand Down Expand Up @@ -75,7 +73,7 @@ static int mqtt_read(struct Network *n, unsigned char *buf, int len,
uint32_t send_time = ztimer_now(ZTIMER_MSEC) + timeout_ms;
do {
rc = sock_tcp_read(&n->sock, _buf, _len, _timeout);
if (rc == -EAGAIN) {
if ((rc == -EAGAIN) || (rc == -ETIMEDOUT)) {
rc = 0;
}

Expand Down