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

at86rf2xx radio driver does not report whether a TX was ACKed #21763

Closed
markus-becker-tridonic-com opened this issue Jan 8, 2020 · 6 comments · Fixed by #21773
Closed

at86rf2xx radio driver does not report whether a TX was ACKed #21763

markus-becker-tridonic-com opened this issue Jan 8, 2020 · 6 comments · Fixed by #21773
Assignees
Labels
bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug

Comments

@markus-becker-tridonic-com
Copy link
Contributor

Describe the bug
The newly introduced at86rf2xx driver is doing automatic retransmissions in hardware based on whether ACKs are received or not. Currently the driver is not invoking ieee802154_radio_handle_ack() as other drivers are doing and required by OpenThread since 4fe1da9.

Impact
OpenThread does not work on this radio.

Environment (please complete the following information):

Additional context
@nandojve, I am in the process of mimic'ing the behaviour of drivers/ieee802154/ieee802154_kw41z.c handle_ack().

@markus-becker-tridonic-com markus-becker-tridonic-com added the bug The issue is a bug, or the PR is fixing a bug label Jan 8, 2020
@jukkar
Copy link
Member

jukkar commented Jan 8, 2020

@nandojve Could you add yourself to CODEOWNERS file as an owner for the rf2xx driver and send a PR for that? This way we know who is maintaining the driver.

@markus-becker-tridonic-com
Copy link
Contributor Author

--- a/drivers/ieee802154/ieee802154_rf2xx.c
+++ b/drivers/ieee802154/ieee802154_rf2xx.c
@@ -215,6 +215,11 @@ static void rf2xx_thread_main(void *arg)
                        k_timer_start(&ctx->trx_isr_timeout, K_MSEC(10), 0);
                } else if (isr_status & (1 << RF2XX_TRX_END)) {
                        if (ctx->trx_state == RF2XX_TRX_PHY_BUSY_RX) {
+                               /* Ensures that automatically ACK will be sent when
+                                * requested */
+                               while (rf2xx_iface_reg_read(dev, RF2XX_TRX_STATUS_REG) ==
+                                       RF2XX_TRX_PHY_STATUS_BUSY_RX_AACK);
+
                                /* Set PLL_ON to avoid transceiver receive
                                 * new data until finish reading process
                                 */
@@ -382,6 +387,48 @@ static int rf2xx_filter(struct device *dev,
        return -ENOTSUP;
 }
 
+#define ACK_FRAME_LEN 3
+#define ACK_FRAME_TYPE (2 << 0)
+#define ACK_FRAME_PENDING_BIT (1 << 4)
+
+static void handle_ack(struct rf2xx_context *ctx, u8_t seq_number)
+{
+       struct net_pkt *ack_pkt;
+       u8_t ack_psdu[ACK_FRAME_LEN];
+
+       ack_pkt = net_pkt_alloc_with_buffer(ctx->iface, ACK_FRAME_LEN,
+                                           AF_UNSPEC, 0, K_NO_WAIT);
+       if (!ack_pkt) {
+               LOG_ERR("No free packet available.");
+               return;
+       }
+
+       /* Re-create ACK frame. */
+       ack_psdu[0] = ACK_FRAME_TYPE; //kw41z_context_data.frame_pending ?
+                     //ACK_FRAME_TYPE | ACK_FRAME_PENDING_BIT : ACK_FRAME_TYPE;
+       ack_psdu[1] = 0;
+       ack_psdu[2] = seq_number;
+
+       if (net_pkt_write(ack_pkt, ack_psdu, sizeof(ack_psdu)) < 0) {
+               LOG_ERR("Failed to write to a packet.");
+               goto out;
+       }
+
+       /* Use some fake values for LQI and RSSI. */
+       (void)net_pkt_set_ieee802154_lqi(ack_pkt, 80);
+       (void)net_pkt_set_ieee802154_rssi(ack_pkt, -40);
+
+       net_pkt_cursor_init(ack_pkt);
+
+       if (ieee802154_radio_handle_ack(ctx->iface, ack_pkt) != NET_OK) {
+               LOG_INF("ACK packet not handled - releasing.");
+       }
+
+out:
+       net_pkt_unref(ack_pkt);
+}
+
+
 static int rf2xx_tx(struct device *dev,
                    struct net_pkt *pkt,
                    struct net_buf *frag)
@@ -437,6 +484,11 @@ static int rf2xx_tx(struct device *dev,
        case RF2XX_TRX_PHY_STATE_TRAC_INVALID:
                response = -EINTR;
                break;
+
+       case RF2XX_TRX_PHY_STATE_TRAC_SUCCESS:
+               response = 0;
+               handle_ack(ctx, frag->data[2]);
+               break;
        /* RF2XX_TRX_PHY_STATE_TRAC_SUCCESS:
         *  The transaction was responded to by a valid ACK, or, if no
         *  ACK is requested, after a successful frame transmission.

the change above allows me to do a full Thread Commissioning using the rf2xx driver.

@nandojve
Copy link
Member

nandojve commented Jan 8, 2020

@markus-becker-tridonic-com thank you for your help with RF2xx driver for OT. I did little changes on your proposal. If you have time to help testing we can conclude this soon. Please take look at #21773.

@jukkar jukkar added the priority: medium Medium impact/importance bug label Jan 14, 2020
nandojve added a commit to nandojve/zephyr that referenced this issue Jan 14, 2020
The rf2xx driver is doing automatic retransmissions in hardware based
on whether ACKs are received or not. Currently the driver is not
invoking ieee802154_radio_handle_ack() as other drivers are doing and
required by OpenThread since 4fe1da9. This add rf2xx_handle_ack method
to ensures required ACK processing when driver performs TX.

Fixes: zephyrproject-rtos#21763

Signed-off-by: Gerson Fernando Budke <nandojve@gmail.com>
@markus-becker-tridonic-com
Copy link
Contributor Author

@nandojve do you think this could still make it into the 2.2 release?

@nandojve
Copy link
Member

@nandojve do you think this could still make it into the 2.2 release?

Hi @markus-becker-tridonic-com, the solution is ready at #21773 and approved. In this case I'm struggled, travelling until end of month. So, I don't have conditions to setup OT environment to prove the solution until early Feb. If you have conditions to test and share test results showing everything is OK, probably solution will be merged closing the issue.

@markus-becker-tridonic-com
Copy link
Contributor Author

@nandojve I have set-up FRDM-K64F with ATREB233-XPRO, the driver and OpenThread work fine and connect to our Thread certified BorderRouter.

@markus-becker-tridonic-com markus-becker-tridonic-com changed the title at86rf2xx radio driver does not report whether a TX was a ACKed at86rf2xx radio driver does not report whether a TX was ACKed Jan 28, 2020
@jukkar jukkar added the has-pr label Jan 28, 2020
jukkar pushed a commit that referenced this issue Jan 29, 2020
The rf2xx driver is doing automatic retransmissions in hardware based
on whether ACKs are received or not. Currently the driver is not
invoking ieee802154_radio_handle_ack() as other drivers are doing and
required by OpenThread since 4fe1da9. This add rf2xx_handle_ack method
to ensures required ACK processing when driver performs TX.

Fixes: #21763

Signed-off-by: Gerson Fernando Budke <nandojve@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants