Skip to content

gnrc/[gomach,lwmac]: Remove unused queue size option#14316

Merged
miri64 merged 2 commits into
RIOT-OS:masterfrom
leandrolanzieri:pr/gnrc/gomach_lwmac_remove_queue_size_option
Jun 19, 2020
Merged

gnrc/[gomach,lwmac]: Remove unused queue size option#14316
miri64 merged 2 commits into
RIOT-OS:masterfrom
leandrolanzieri:pr/gnrc/gomach_lwmac_remove_queue_size_option

Conversation

@leandrolanzieri
Copy link
Copy Markdown
Contributor

Contribution description

This removes the IPC message queue size configuration options from GoMacH and LWMAC modules as they are not used:

  • CONFIG_GNRC_GOMACH_IPC_MSG_QUEUE_SIZE_EXP
  • GNRC_GOMACH_IPC_MSG_QUEUE_SIZE
  • CONFIG_GNRC_LWMAC_IPC_MSG_QUEUE_SIZE_EXP
  • GNRC_LWMAC_IPC_MSG_QUEUE_SIZE

Testing procedure

  • Green CI
  • Test applications should compile and work as usual as the parameters are not used at all

Issues/PRs references

Spotted while reviewing #14104

@leandrolanzieri leandrolanzieri added Area: network Area: Networking Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation labels Jun 19, 2020
@leandrolanzieri leandrolanzieri added this to the Release 2020.07 milestone Jun 19, 2020
@leandrolanzieri leandrolanzieri added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jun 19, 2020
fjmolinas
fjmolinas previously approved these changes Jun 19, 2020
Copy link
Copy Markdown
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

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

Git grep shows no occurrence other than the ones removed in this pr. ACK

git grep GNRC_GOMACH_IPC_MSG_QUEUE_SIZE
sys/include/net/gnrc/gomach/gomach.h:#ifndef CONFIG_GNRC_GOMACH_IPC_MSG_QUEUE_SIZE_EXP
sys/include/net/gnrc/gomach/gomach.h:#define CONFIG_GNRC_GOMACH_IPC_MSG_QUEUE_SIZE_EXP        (3U)
sys/include/net/gnrc/gomach/gomach.h:#ifndef GNRC_GOMACH_IPC_MSG_QUEUE_SIZE
sys/include/net/gnrc/gomach/gomach.h:#define GNRC_GOMACH_IPC_MSG_QUEUE_SIZE  (1 << CONFIG_GNRC_GOMACH_IPC_MSG_QUEUE_SIZE_EXP)

@fjmolinas fjmolinas dismissed their stale review June 19, 2020 09:17

Seems like they where unused in the original PR as well, might we be missing something?

@fjmolinas
Copy link
Copy Markdown
Contributor

I took a look at the original PR #5618 makes me think at first it used some quind of custom message queue that was later adapted to use pktsnip, and therefore maybe the define became unused, maybe @miri64 has an idea.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Jun 19, 2020

I took a look at the original PR #5618 makes me think at first it used some quind of custom message queue that was later adapted to use pktsnip, and therefore maybe the define became unused, maybe @miri64 has an idea.

No, these are macros for the IPC message queue. The modules where implemented right in the transition period of gnrc_netif, so I can imagine, that originally they were intended to be used with msg_init_queue() in a dedicated lwmac/gomach thread. However, they now use the gnrc_netif queue definitions, so I guess they really are not needed.

Copy link
Copy Markdown
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

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

ACK from my side. Both protocols still build when used with examples/gnrc_networking_mac for samr21-xpro

@miri64 miri64 merged commit 3a135c9 into RIOT-OS:master Jun 19, 2020
@leandrolanzieri leandrolanzieri deleted the pr/gnrc/gomach_lwmac_remove_queue_size_option branch June 19, 2020 11:25
@leandrolanzieri
Copy link
Copy Markdown
Contributor Author

Thanks for the review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants