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

Scan needs NcpState to be ON #2778

Merged
merged 1 commit into from
Jun 3, 2024
Merged

Scan needs NcpState to be ON #2778

merged 1 commit into from
Jun 3, 2024

Conversation

keeramis
Copy link
Contributor

@keeramis keeramis commented May 31, 2024

Problem

For a brand new factory fresh P2, joining a new network via control requests throws an error.

Solution

It appears that the joinNewNetwork for realtek performs a scan that requires the NcpState to be ON. There is nothing that's turning the NcpState ON so I added in that functionality.

Open question

Is the scan call needed and why, and if the flow can be simplified?

Steps to Test

  1. Clear all wifi networks on the device
  2. Reset the device
  3. Attempt to join a new network

Latest Particle-CLI branch can help perform the above steps. See particle wifi xxx commands.

Example App

void setup() {
  /* A minimal example app is super helpful 
   * for testing new features and fixes. 
   * A link to a Docs PR is even better!
   */
}

void loop() {

}

References

Links to the Community, Docs, Other Issues, etc..


Completeness

  • User is totes amazing for contributing!
  • Contributor has signed CLA (Info here)
  • Problem and Solution clearly stated
  • Run unit/integration/application tests on device
  • Added documentation
  • Added to CHANGELOG.md after merging (add links to docs and issues)

@avtolstoy avtolstoy self-requested a review May 31, 2024 12:04
Copy link
Member

@avtolstoy avtolstoy left a comment

Choose a reason for hiding this comment

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

👍

@scott-brust
Copy link
Member

Is the scan call needed and why, and if the flow can be simplified?

The scan is used to detect what security the actual AP is using. We can remove it if we want to rely entirely on the supplied security type from the control request. I think at some point in the past, this field was not set correctly all of the time, resulting in a situation where we did not know the security type. I think this is fixed now though, as we rely on that field when connecting to hidden networks.

We could probably remove it? But it may also be silently working if people are supplying the wrong security type

@keeramis keeramis marked this pull request as ready for review May 31, 2024 15:44
@keeramis
Copy link
Contributor Author

I am merging this PR

@keeramis keeramis added this to the 5.8.2 milestone May 31, 2024
@keeramis
Copy link
Contributor Author

@scott-brust I like that the security type is detected. 🤷‍♀️ Open to opinions

@keeramis
Copy link
Contributor Author

keeramis commented Jun 3, 2024

This is the current behavior with the security types and the inevitable scanning:

For RTK, scan is run regardless for joinNewNetwork and not for joinKnownNetwork. joinKnownNetwork does not go through the scanning process and trusts whatever is stored in the filesystem /sys/wifi_config.bin

For joinNewNetwork:

  • If the control request provides security as NONE, the joinNewNetwork will update the security -in the filesystem as well for the joining process- from the result of the scanning process. This is regardless of network being hidden or not.
  • If the control request provides security but the network is hidden, the joinNewNetwork will update the security -in the filesystem as well for the joining process- from the result of the scanning process. This is probably because device-os wants to better support hidden networks to ensure the correct security type is used.

From the CLI side, we never pass the hidden network flag in particle-usb https://github.com/particle-iot/particle-usb/blob/main/src/wifi-device.js#L73-L97 .

@keeramis keeramis merged commit 8281cdf into develop Jun 3, 2024
13 checks passed
@keeramis keeramis deleted the fix/p2-join-new-network branch June 3, 2024 15:59
@scott-brust scott-brust mentioned this pull request Jun 13, 2024
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.

3 participants