ieee802154/submac: calculate symbol time on demand

[jia200x]

Contribution description

This PR adds a mechanism to calculate the symbol time of IEEE 802.15.4 PHYs on runtime. This is required to calculate the ACK Timeout for all PHYs.

So far the SubMAC has been hardcoded to use 2.4 GHz O-QPSK values, which rule-out SubGHz radios. With this, it should be possible to run those.

Testing procedure

Test examples/gnrc_networking and tests/ieee802154_submac. Everything should work as expected.

Issues/PRs references

Besides other IEEE 802.15.4 PHYs, this would enable the use case of IEEE 802.15.4 over LoRa (see #19172)

[jia200x]

@benpicco it seems the zep_dispatcher won't compile when I add DEBUG to ieee802154.c. Is there any macro I could use to guard that?

[benpicco]

Is channel page really enough to determine the symbol time?
If I understand it correctly, all 802.15.4g-2012 modulations are mapped to page 9 & 10

so channel page gives you no additional info. And for LoRa there isn't even a channel page AFAIK.

I think we won't get around a custom struct here.

typedef struct {
    bool subghz;
    bool high_rate;
} ieee802154_oqpsk_params_t;

typedef struct {
    uint8_t chips;
    uint8_t rate_mode;
} ieee802154_mr_oqpsk_params_t;

…

uint32_t ieee802154_get_symbol_time(ieee802154_phy_mode_t phy, const void *params);

mind you that AFAIK for MR-FSK the preamble length also plays into the ACK/CSMA delay, not just the symbol time.

So we might need two functions

uint32_t ieee802154_get_csma_backoff_us(ieee802154_phy_mode_t phy, const void *params);

uint32_t ieee802154_get_ack_turnaround_us(ieee802154_phy_mode_t phy, const void *params);

[jeandudey]

Is channel page really enough to determine the symbol time?

Not really but helps to identify which is the PHY used at the moment, for MR-FSK its a bit more complex since you need to have sort of a database of the standard defined PHY configurations and also the generic ones for page 10 that the standard allows to specify.

I think it's best to remove the ieee802154_phy_mode_t enum and use the channel page for that. However, I think there's still a need for a MR PHY modes enum since those share the page 9.

[jeandudey]

Which reminds me we need a way to handle the special cases of pages 9 and 10. Not sure there's a need for page 10 but could be useful.

[benpicco]

But this is only internal, we don't have to use channel page if it complicates things.
Using our PHY type enum allows to also use this for non-IEEE 802.15.4 PHYs (and lets us use any mode combination, also the non-standard high-data rate modes implemented on some classic 802.15.4 radios).

[jia200x]

And for LoRa there isn't even a channel page AFAIK.

There's no channel page for LoRa, but there are already use cases for IEEE 802.15.4 over LoRa. We could just pick a custom channel page that doesn't conflict with the standard.

I think we won't get around a custom struct here.

Is there a way to abstract this information into a set of common input variables? Otherwise I'm afraid it will be really hard to write upper layer code :/.

So we might need two functions

Sure, it makes sense. I would still keep the proposed ieee802154_get_symbol_time function though because slotted MAC layers use the symbol time as common multiple for timers.

[jia200x]

Ah, you are right. Probably is not that hard to go that way, since we will eventually update the config_phy function of the Radio HAL anyway.

We could then kill 2 birds with one stone with this:

diff --git a/sys/include/net/ieee802154/radio.h b/sys/include/net/ieee802154/radio.h
index c8f71afa90..aa3a6e5d6e 100644
--- a/sys/include/net/ieee802154/radio.h
+++ b/sys/include/net/ieee802154/radio.h
@@ -453,11 +453,38 @@ typedef enum {
  * @brief Holder of the PHY configuration
  */
 typedef struct {
-    ieee802154_phy_mode_t phy_mode; /**< IEEE802.15.4 PHY mode */
+    typedef union {
+#ifdef MODULE_IEEE802154_OQPSK
+        struct {
+            bool subghz;
+            bool high_rate;
+        } oqpsk_params;
+#endif
+#ifdef MODULE_IEEE802154_MR_OQPSK
+        struct {
+            uint8_t chips;
+            uint8_t rate_mode;
+        } mr_oqpsk_params;
+#endif
+#ifdef MODULE_IEEE802154_MR_ODFM
+        struct {
+            uint8_t option;
+            uint8_t msc;
+        } mr_odfm_params;
+#endif
+#ifdef MODULE_IEEE802154_MR_FSK
+        struct {
+            uint8_t rate;
+            uint8_t mod_idx
+            uint8_t mod_order;
+            uint8_t preamble_len;
+            bool fec;
+        } mr_fsk_params;
+#endif
     uint16_t channel;               /**< IEEE802.15.4 channel number */
     uint8_t page;                   /**< IEEE802.15.4 channel page */
     int8_t pow;                     /**< TX power in dBm */
-} ieee802154_phy_conf_t;
+} ieee802154_phy_pib_t;
 
 /**
  * @brief IEEE 802.15.4 radio operations
@@ -686,7 +713,7 @@ struct ieee802154_radio_ops {
      * @return -EINVAL  if the configuration is not valid for the device.
      * @return <0       error, return value is negative errno indicating the cause.
      */
-    int (*config_phy)(ieee802154_dev_t *dev, const ieee802154_phy_conf_t *conf);
+    int (*config_phy)(ieee802154_dev_t *dev, const ieee802154_phy_pib_t *conf);
 
     /**
      * @brief Set number of frame retransmissions
@@ -915,7 +942,7 @@ static inline int ieee802154_radio_set_cca_mode(ieee802154_dev_t *dev,
  * @return result of @ref ieee802154_radio_ops::config_phy
  */
 static inline int ieee802154_radio_config_phy(ieee802154_dev_t *dev,
-                                              const ieee802154_phy_conf_t *conf)
+                                              const ieee802154_phy_pib_t *conf)
 {
     return dev->driver->config_phy(dev, conf);
 }
diff --git a/sys/include/net/ieee802154/submac.h b/sys/include/net/ieee802154/submac.h
index 3890ab7579..42f0d934f0 100644
--- a/sys/include/net/ieee802154/submac.h
+++ b/sys/include/net/ieee802154/submac.h
@@ -196,15 +196,13 @@ struct ieee802154_submac {
     ieee802154_csma_be_t be;            /**< CSMA-CA backoff exponent params */
     bool wait_for_ack;                  /**< SubMAC is waiting for an ACK frame */
     uint16_t panid;                     /**< IEEE 802.15.4 PAN ID */
-    uint16_t channel_num;               /**< IEEE 802.15.4 channel number */
-    uint8_t channel_page;               /**< IEEE 802.15.4 channel page */
     uint8_t retrans;                    /**< current number of retransmissions */
     uint8_t csma_retries_nb;            /**< current number of CSMA-CA retries */
     uint8_t backoff_mask;               /**< internal value used for random backoff calculation */
     uint8_t csma_retries;               /**< maximum number of CSMA-CA retries */
     int8_t tx_pow;                      /**< Transmission power (in dBm) */
-    ieee802154_fsm_state_t fsm_state;    /**< State of the SubMAC */
-    ieee802154_phy_mode_t phy_mode;     /**< IEEE 802.15.4 PHY mode */
+    ieee802154_fsm_state_t fsm_state;   /**< State of the SubMAC */
+    ieee802154_phy_pib_t pib;           /**< IEEE 802.15.4 Physical Information Base */
     const iolist_t *psdu;               /**< stores the current PSDU */
 };
 

We would need to adapt ieee802154_set_phy_conf though to make it more generic.

[jia200x]

please correct me if I'm wrong: do all IEEE 802.15.4 channel pages use a channel number to identify the channel?

[benpicco]

do all IEEE 802.15.4 channel pages use a channel number to identify the channel?

Yes. (But I still don't understand what the channel pages are actually used for)

[benpicco]

I implemented the suggested approach in #19668

[kfessel]

do all IEEE 802.15.4 channel pages use a channel number to identify the channel?

Yes. (But I still don't understand what the channel pages are actually used for)

isn't the page used to define how to fit channels in some regulatory frequency's and the channel then identify which of them is used

eg 350MHz to 358MHz fits 7 1MHZ channels but you might want to evenly spread 5 to have some wiggle room left an right of each channel

another modulation might be slimmer and you can fit 6 channels

bluetooth also has more channels than wifi (should have used that example)

[benpicco]

it seems the zep_dispatcher won't compile when I add DEBUG to ieee802154.c. Is there any macro I could use to guard that?

Ah that's because debug.h includes thread.h which is does not exist on Linux.
A simple patch would be

diff --git a/core/lib/include/debug.h b/core/lib/include/debug.h
index 4d93e3eee6..fd431cdd4d 100644
--- a/core/lib/include/debug.h
+++ b/core/lib/include/debug.h
@@ -25,8 +25,10 @@
 #define DEBUG_H
 
 #include <stdio.h>
+#ifdef RIOT_VERSION
 #include "sched.h"
 #include "thread.h"
+#endif
 
 #ifdef __cplusplus
 extern "C" {
@@ -42,7 +44,7 @@ extern "C" {
  * information to stdout after verifying the stack is big enough. If `DEVELHELP`
  * is not set, this check is not performed. (CPU exception may occur)
  */
-#ifdef DEVELHELP
+#if defined(DEVELHELP) && defined(RIOT_VERSION)
 #include "cpu_conf.h"
 #define DEBUG_PRINT(...) \
     do { \

[kYc0o]

Thanks for the PR! I'll test asap on my sub-GHz hardware. I'll try to review but I may take some time.

If it needs specific testing just let me know.

[fabian18]

I was wondering where this value comes from and found an explanation here.
Also helpful may be "6.3 PPDU format" (in 802.15.4-2006) to understand where the number of symbols comes from and the formula for macAckWaitDuration.
May it be that we would have to consider a different number of symbols for 868Mhz or is this neglectable for us?

[jeandudey]

IIRC macAckWaitDuration also has to be calculated at runtime since some PHYs can have different values for the ACK timeout. But on 802.15.4-2020 there's no formula for the ACK timeout and can be chosen freely I think.

[benpicco]

Yea the standard is unnecessary cryptic here, you have to piece information together from multiple tables and have yourself asking if you got it right or not. I'm still not 100% sure the formulas used in the at86rf215 driver are correct.

The most enlightening was the 'diff' contained in the IEEE 802.15.4-2012g amendment:

[mguetschow]

Can this now be closed after #19668 is in?