Skip to content

Disable AP mode after we're configured and connected to WiFi #41

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

Closed
wants to merge 5 commits into from

Conversation

JCBird1012
Copy link

Hi!

Earlier today, I noticed the TRONBYT-CONFIG network was sticking around, even though my Tronbyt was connected to WiFi and actively fetching images. Since TRONBYT-CONFIG is an unsecured network, this effectively allows anyone to re-configure the Tronbyt to reconnect somewhere else, which is not ideal.

This PR changes a few things to prevent that.

Stops the open AP once we're connected to a network
As soon as we get an IP (in IP_EVENT_STA_GOT_IP), we call esp_netif_dhcps_stop() and switch to WIFI_MODE_STA so that the unsecured TRONBYT-CONFIG SSID disappears.

Only fall back to an AP when we're disconnected from WiFi or fail to connect
If the STA interface fails to reconnect after MAX_RECONNECT_ATTEMPTS, we switch to AP-only mode (not AP+STA) so that the device can still be re-configured - the STA interface stays down until new credentials are submitted, since there's no point in leaving it on - we're not actively connecting because at that point, we've exceeded MAX_RECONNECT_ATTEMPTS and have given up.

Re-enable AP+STA on manual re-configuration via the web UI
When the user submits new WiFi settings via the web UI, go back to WIFI_MODE_APSTA, then attempt to join the target network. If it succeeds, the AP drops again. In reality, we could probably just go to STA mode here, but I wanted to leave the avenue open (in the future) for immediate feedback via the web UI on whether connecting with the entered credentials was successful or not. If we switched to STA immediately here, we'd lose the option to potentially send one last response via the web UI if we were actually able to connect or not.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Summary of Changes

Hello @JCBird1012, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request implements changes to the WiFi management logic to enhance security and improve the user experience during connection failures. The primary goal is to disable the open configuration AP once the device is successfully connected to a secure network, while providing clear fallback mechanisms and visual feedback when connectivity is lost or requires manual intervention.

Highlights

  • Security Improvement: The open configuration Access Point (AP) is now disabled automatically once the device successfully connects to a configured WiFi network, switching to STA-only mode. This prevents unauthorized re-configuration after setup.
  • Connection Failure Fallback: If the device fails to connect to the configured WiFi after a maximum number of attempts, it now explicitly switches to AP-only mode (instead of AP+STA) to allow re-configuration via the TRONBYT-CONFIG network.
  • Manual Re-configuration: When new WiFi credentials are submitted via the web UI, the device temporarily switches back to AP+STA mode before attempting the connection. This allows the AP to remain active during the connection attempt, potentially enabling future feedback mechanisms via the web UI.
  • Visual Feedback: The configuration screen (showing the AP details) is now displayed whenever the device disconnects from WiFi, providing a visual indicator that it may be in a state requiring re-configuration.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configureGemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request significantly improves the security of the Tronbyt device by disabling the unsecured configuration AP (TRONBYT-CONFIG) once it's successfully connected to a WiFi network. The changes also refine the fallback behavior: the AP is re-enabled in AP-only mode if the device fails to connect to the configured WiFi after several attempts, allowing for re-configuration. When new settings are submitted, the device temporarily enters AP+STA mode to facilitate the connection attempt, with plans for future UI feedback.

The code changes are well-structured, with clear logic for managing WiFi states and AP activity. The introduction of helper functions like ap_is_running and show_config_screen enhances modularity. Logging is generally informative, and error handling for critical operations is appropriate.

One suggestion is provided to ensure the AP's DHCP server is explicitly started in save_handler to fully support the intended AP+STA mode functionality for potential UI feedback. Overall, this is a solid improvement to the device's network management and security.

@JCBird1012
Copy link
Author

JCBird1012 commented Jun 13, 2025

Some rough logic I wrote to help me keep track -

  1. AP+STA on startup (good!)
  2. STA only once we've connected to WiFi
  3. AP only if we've lost WiFi connection (which only happens when we've given up on reconnecting, so STA is pointless the only way to reset retries is by power-cycling or entering new credentials via the web UI - might be worth tweaking this so we'll at least try again at some point on a timer)
  4. AP+STA again once the user has re-entered credentials (STA to try connecting again, but AP so we don't immediately disconnect the user from the web UI)
  5. STA if that connection succeeds... and then by virtue back to AP only if we can't connect with the user's entered credentials (original logic in point 3)

I'm not a C-dev so it was bound to get me at some point.
@JCBird1012 JCBird1012 deleted the branch tronbyt:main June 14, 2025 21:16
@JCBird1012 JCBird1012 closed this Jun 14, 2025
@JCBird1012 JCBird1012 deleted the main branch June 14, 2025 21:16
@JCBird1012 JCBird1012 restored the main branch June 14, 2025 21:17
@JCBird1012 JCBird1012 reopened this Jun 14, 2025
@tavdog
Copy link
Member

tavdog commented Jun 17, 2025

I just noticed this after writing some of this logic myself today. I'll check it out.

@tavdog
Copy link
Member

tavdog commented Jun 17, 2025

Thanks for your work on this. I tested your code and it works well. The only problem is that it kills the AP too soon. We need to have it stick around for a least few minutes after boot to allow for a re-config without having to bring the Tronbyt out of current wifi reception.

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

Successfully merging this pull request may close these issues.

2 participants