WifiConnectionManager: Scan timer reconfig connect

[Zclarkwilliams]

Description

When the Wifi network is enabled the connection
manager will trigger a network scan without a
profile to use. If there is a connected network
or attempting a connection, the scan will
interrupt and break the connection.

Fix - The Wifi Connection Manager will register
the scan on timer tick but will not set the timer. This timer will only be set when the user enters or selects a profile for connection in the BIOS menu. If the user does not select a profile there is no
need to start a scan timer. Additionally the scan
on timer tick will check for a profile to connect
and if no profile found then cancel the timer and
exit.

• Breaking change?
  • Breaking change - Does this PR cause a break in build or boot behavior?
  • Examples: Does it add a new library class or move a module to a different repo.
• Impacts security?
  • Security - Does this PR have a direct security impact?
  • Examples: Crypto algorithm change or buffer overflow fix.
• Includes tests?
  • Tests - Does this PR include any explicit test code?
  • Examples: Unit tests or integration tests.

How This Was Tested

This change was tested on platforms with Wifi connections established and tested if this fixes the break in the current connect as well if this change breaks anything when the platform is supposed to scan correctly.

Integration Instructions

N/A

[mdkinney]

@ardbiesheuvel You had comments on the other PR #11039 for this issue that was closed and this alternate approach was implemented. Do you have comments on this approach?

[eblipp]

This pull request makes several changes to the WiFi Connection Manager in the EDK II project, with a focus on improving timer management, connection logic, and error handling. Here’s a concise review of the changes:

Summary of Changes

1. Logic Fixes for Connection Flow

• In WifiConnectionMgrDriver.c, the logic for handling connection status is updated:
  • Previously, the code checked for a successful connection (!EFI_ERROR(Status)) before jumping to error handling (goto ERROR1).
  • Now, it checks for an error (EFI_ERROR(Status)), which is a more standard approach and likely fixes unintended behavior.

2. Improved Timer Management

• In multiple locations (WifiConnectionMgrDriver.c, WifiConnectionMgrHiiConfigAccess.c, WifiConnectionMgrImpl.c), the code now:
  • Sets periodic timers only when necessary (e.g., when a profile is available).
  • Cancels timers when not needed, such as after a successful connection or when there’s no profile to connect to.
  • Adds error handling: if setting a timer fails, it closes the event to avoid resource leaks.

3. Refined Scan and Connection State Logic

• The timer tick handler in WifiConnectionMgrImpl.c now cancels the timer and returns early if there’s no profile to connect to, with a debug message for visibility.
• The scan trigger logic is updated to:
  • Avoid unnecessary scans when already connected or in the process of connecting.
  • Reset scan request flags and timing correctly.

Code Quality / Style

• Error handling and resource management are improved throughout—timers are canceled and events are closed when needed.
• Comments are added to explain the intent of new logic.
• The changes bring the code closer to standard UEFI practices for timer/event handling and state management.

Impact

• Reliability & Efficiency:
  These changes should reduce unnecessary timer events, improve connection reliability, and prevent resource leaks.
• Readability:
  The code is easier to follow with the added comments and logical flow improvements.
• Debugging & Maintenance:
  Added debug messages and clearer state transitions make future troubleshooting easier.

Suggestions

• Consider further refactoring to centralize timer management to avoid repetition.
• Ensure thorough testing for edge cases, such as rapid connect/disconnect scenarios and timer failures.

Verdict

Positive:
This PR improves robustness, correctness, and maintainability of the WiFi Connection Manager. I recommend merging after confirming that the updated timer and connection logic works as expected across all supported platforms and connection scenarios.

---

If you need a deeper review of the specific logic or want to discuss edge cases, let me know!