Skip to content

Add missing config options to kconfig #60

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

h2zero
Copy link

@h2zero h2zero commented May 11, 2025

When using in an esp-idf environment some options are missing and need to be set on the command line to override. This adds the missing options so that they can be set in menuconfig/sdkconfig.defaults.

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds missing configuration options to the Kconfig.projbuild file to allow overriding options via menuconfig/sdkconfig.defaults in an esp-idf environment.

  • Added configuration for Async TCP stack size
  • Introduced configurations for Async TCP event queue size, max ACK time, and task priority

@h2zero h2zero requested a review from Copilot May 11, 2025 22:36
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds missing configuration options to the Kconfig.projbuild file to support setting these values within menuconfig/sdkconfig.defaults in an ESP-IDF environment.

  • Added AsyncTCP configuration options: task stack size, event queue size, maximum ACK time, and task priority.

When using in an esp-idf environment some options are missing and need to be set on the command line to override. This adds the missing options so that they can be set in menuconfig/sdkconfig.defaults.
@h2zero h2zero requested a review from Copilot May 11, 2025 23:04
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds new configuration options to help override missing settings in the esp-idf environment for AsyncTCP.

  • Introduces ASYNC_TCP_STACK_SIZE for configuring the task stack size.
  • Adds ASYNC_TCP_QUEUE_SIZE, ASYNC_TCP_MAX_ACK_TIME, and ASYNC_TCP_PRIORITY with descriptive help texts to support configuration via menuconfig/sdkconfig.defaults.

@@ -1,5 +1,11 @@
menu "AsyncTCP Configuration"

config ASYNC_TCP_STACK_SIZE
Copy link
Preview

Copilot AI May 11, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider clarifying in the help text that the stack size is specified in bytes to avoid any ambiguity.

Copilot uses AI. Check for mistakes.

Comment on lines +40 to +42
Configures the size of the Async TCP event queue. Lowering the value will reduce resource use
but will limit the number of events that can be processed. Increasing will allow for more
connections/event to be handled.
Copy link
Preview

Copilot AI May 11, 2025

Choose a reason for hiding this comment

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

[nitpick] It might be beneficial to specify the expected units or provide more detail on the range of acceptable values for ASYNC_TCP_QUEUE_SIZE.

Suggested change
Configures the size of the Async TCP event queue. Lowering the value will reduce resource use
but will limit the number of events that can be processed. Increasing will allow for more
connections/event to be handled.
Configures the size of the Async TCP event queue (number of events). Lowering the value will reduce
resource use but will limit the number of events that can be processed. Increasing the value will
allow for more connections/events to be handled. Recommended range: 16 to 256. Default: 64.

Copilot uses AI. Check for mistakes.

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

Successfully merging this pull request may close these issues.

2 participants