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

tests: rework ieee802154 drivers #17838

Merged
merged 18 commits into from
Jul 11, 2022

Conversation

MrKevinWeiss
Copy link
Contributor

@MrKevinWeiss MrKevinWeiss commented Mar 22, 2022

Contribution description

Split from #17789, but with a difference: instead of using symlinks the common functionality to interact with ieee802154 netdev drivers is implemented in its own test_utils module.

Testing procedure

  • Run the tests, you should be able to send an receive Ethernet frames
  • Green CI

Issues/PRs references

Split from #17789
Similar to #17813

@github-actions github-actions bot added Area: boards Area: Board ports Area: CI Area: Continuous Integration of RIOT components Area: doc Area: Documentation Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration Area: network Area: Networking Area: sys Area: System Area: tests Area: tests and testing framework labels Mar 22, 2022
@MrKevinWeiss MrKevinWeiss changed the title Pr/tests/154 drivers rework tests: rework ieee802154 drivers Mar 22, 2022
@MrKevinWeiss MrKevinWeiss added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 22, 2022
@MrKevinWeiss MrKevinWeiss force-pushed the pr/tests/154_drivers_rework branch from 23719fc to 461c045 Compare March 23, 2022 11:58
@github-actions github-actions bot removed the Area: CI Area: Continuous Integration of RIOT components label Mar 23, 2022
Copy link
Contributor

@leandrolanzieri leandrolanzieri left a comment

Choose a reason for hiding this comment

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

Test applications that have device-specific shell commands should keep those. Also, please take a look at the initial puts of the test applications, as many say Ethernet driver.

drivers/include/kw2xrf.h Outdated Show resolved Hide resolved
drivers/include/net/netdev.h Outdated Show resolved Hide resolved
drivers/kw2xrf/Kconfig Outdated Show resolved Hide resolved
sys/include/test_utils/netdev_ieee802154_minimal.h Outdated Show resolved Hide resolved
tests/driver_at86rf2xx/main.c Outdated Show resolved Hide resolved
tests/driver_at86rf215/main.c Outdated Show resolved Hide resolved
tests/driver_kw2xrf/main.c Outdated Show resolved Hide resolved
@MrKevinWeiss MrKevinWeiss force-pushed the pr/tests/154_drivers_rework branch from 461c045 to da892ca Compare March 24, 2022 13:19
@github-actions github-actions bot removed Area: boards Area: Board ports Area: Kconfig Area: Kconfig integration labels Mar 24, 2022
Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for the clean up. Please squash right in the style nit-picks.

sys/include/test_utils/netdev_ieee802154_minimal.h Outdated Show resolved Hide resolved
sys/test_utils/netdev_ieee802154_minimal/shell_commands.c Outdated Show resolved Hide resolved
sys/test_utils/netdev_ieee802154_minimal/shell_commands.c Outdated Show resolved Hide resolved
sys/test_utils/netdev_ieee802154_minimal/shell_commands.c Outdated Show resolved Hide resolved
sys/test_utils/netdev_ieee802154_minimal/shell_commands.c Outdated Show resolved Hide resolved
sys/test_utils/netdev_ieee802154_minimal/shell_commands.c Outdated Show resolved Hide resolved
tests/driver_at86rf215/main.c Outdated Show resolved Hide resolved
tests/driver_at86rf2xx/README.md Show resolved Hide resolved
@MrKevinWeiss MrKevinWeiss force-pushed the pr/tests/154_drivers_rework branch from d21f321 to 2eac520 Compare July 5, 2022 15:02
@MrKevinWeiss
Copy link
Contributor Author

Done, thanks for the review @maribu

Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

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

ACK. Code looks good. I tested with tests/driver_at86rf2xx using the samr21-xpro and with tests/driver_at86rf215 using our custom miot-nucleo-f767zi testbed board. There is one issue with the AT86RF215 test application, as that needs to allocate twice the number of netdevs, as the sub-GHz and the 2.4 GHz transceiver in that chip are represented as two netdevs in RIOT. I already tested it successfully with the proposed changes.

#endif

#define AT86RF215_NUM ARRAY_SIZE(at86rf215_params)
#define NETDEV_IEEE802154_MINIMAL_NUMOF AT86RF215_NUM
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#define NETDEV_IEEE802154_MINIMAL_NUMOF AT86RF215_NUM
/* AT86RF215 contains one sub-GHz and one 2.4 GHz driver */
#define NETDEV_IEEE802154_MINIMAL_NUMOF (2 * AT86RF215_NUM)

#include "od.h"

static char batmon_stack[THREAD_STACKSIZE_MAIN];
static at86rf215_t *dev;
static at86rf215_t at86rf215[AT86RF215_NUM];
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
static at86rf215_t at86rf215[AT86RF215_NUM];
static at86rf215_t at86rf215[NETDEV_IEEE802154_MINIMAL_NUMOF];

@MrKevinWeiss
Copy link
Contributor Author

😢

@MrKevinWeiss MrKevinWeiss force-pushed the pr/tests/154_drivers_rework branch from 2eac520 to 37bdc32 Compare July 11, 2022 07:32
@MrKevinWeiss
Copy link
Contributor Author

ping @maribu applied the fixes and rebased.

@MrKevinWeiss MrKevinWeiss merged commit 1ba2ef8 into RIOT-OS:master Jul 11, 2022
@MrKevinWeiss MrKevinWeiss deleted the pr/tests/154_drivers_rework branch July 11, 2022 11:38
@MrKevinWeiss
Copy link
Contributor Author

Many thanks @leandrolanzieri @maribu and @jia200x

@chrysn chrysn added this to the Release 2022.07 milestone Aug 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: doc Area: Documentation Area: drivers Area: Device drivers Area: network Area: Networking Area: sys Area: System Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants