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.

Sign up for GitHub

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

Jump to bottom

Added support to check if SSID is connected #37

Merged
merged 1 commit into from
Jul 5, 2020
Merged

Added support to check if SSID is connected #37

merged 1 commit into from
Jul 5, 2020
+230 −7

Conversation

ErrorErrorError
Copy link
Collaborator

@ErrorErrorError ErrorErrorError commented Jun 30, 2020 •
edited
Loading

#21

  • Added Wi-Fi details on the connected Wi-Fi
  • still missing a few informations regarding ip address, country code, router address, internet and security for WiFi details

@williambj1 williambj1 added the enhancement New feature or request label Jun 30, 2020
@williambj1 williambj1 added this to the v1.0.0 milestone Jun 30, 2020
@zxystd
Copy link
Collaborator

zxystd commented Jun 30, 2020 •
edited by williambj1
Loading

  1. IP address and router address should be fetched in userspace, not from the driver(eg. through ifconfig command you can get these information).
  2. all security-related info is WIP.

Thanks for your contribution.

zxystd
zxystd reviewed Jun 30, 2020
View reviewed changes
ClientKit/Api.c Outdated Show resolved Hide resolved
@williambj1
Copy link
Collaborator

@ErrorErrorError

Huge thanks for your PR, I really appreciate your time and effort.
I kindly request you to adapt to the new code style @igorkulman optimized in #35 while resolving the conflicts.

Thanks in advance! ;)

@igorkulman
Copy link
Contributor

Looking at your PR, it correctly detects which network I am connected. This is great, I have been missing this.

Just the router and IP address look like malformed strings

Screenshot 2020-06-30 at 19 54 07

@ErrorErrorError
Copy link
Collaborator Author

ErrorErrorError commented Jun 30, 2020 •
edited
Loading

Looking at your PR, it correctly detects which network I am connected. This is great, I have been missing this.

Just the router and IP address look like malformed strings

Screenshot 2020-06-30 at 19 54 07

Yup fixed the issue with local ip and router, I just had bssid as a placeholder in the meantime.

Some missing string on items are:

  • Country code
  • Security
  • Internet reachability
  • Channel (Wi-Fi channel width, e.g. 20MHz, 60MHz, 80MHz, ect...)

Other than that everything else is ready for review!

@ErrorErrorError ErrorErrorError changed the title Added support to check if SSID is connected Added support to check if SSID is connected (https://github.com/zxystd/HeliPort/issues/21) Jun 30, 2020
@ErrorErrorError ErrorErrorError changed the title Added support to check if SSID is connected (https://github.com/zxystd/HeliPort/issues/21) Added support to check if SSID is connected Jun 30, 2020
@Goshin
Copy link
Collaborator

Goshin commented Jun 30, 2020

@ErrorErrorError Internet reachability check is hard to implement in the driver layer. I would suggest implement it with a simple HTTP test in HeliPort or just omit this item.

@igorkulman
Copy link
Contributor

@ErrorErrorError Internet reachability check is hard to implement in the driver layer. I would suggest implement it with a simple HTTP test in HeliPort or just omit this item.

On iOS I use https://github.com/ashleymills/Reachability.swift, it also should work on macOS. But it will only check if Internet is available using the default device, not a specific device.

@ErrorErrorError
Copy link
Collaborator Author

@ErrorErrorError Internet reachability check is hard to implement in the driver layer. I would suggest implement it with a simple HTTP test in HeliPort or just omit this item.

On iOS I use https://github.com/ashleymills/Reachability.swift, it also should work on macOS. But it will only check if Internet is available using the default device, not a specific device.

I will check it out and see if I can check reachability using this or just using https.

@ErrorErrorError
Copy link
Collaborator Author

pull the latest code, the bandwidth should come from driver(from station info), more eaiser.

Awesome, will do! Thanks for the changes.

@ErrorErrorError
Copy link
Collaborator Author

I can also get the local ip address using scutil if you believe that's a better implementation than the current one.

@williambj1
Copy link
Collaborator

williambj1 commented Jul 2, 2020 •
edited
Loading

I think the method from 50beb8e and 491c3fe is good enough, well done!

@williambj1 williambj1 linked an issue Jul 2, 2020 that may be closed by this pull request
Improvements for StatusMenu #21
Closed
17 tasks
@igorkulman igorkulman mentioned this pull request Jul 3, 2020
Closed
@EETagent EETagent mentioned this pull request Jul 3, 2020
Closed
@williambj1
Copy link
Collaborator

@ErrorErrorError is this PR ready to be merged? Do we need to wait for APIs for security and country code?

@ErrorErrorError
Copy link
Collaborator Author

ErrorErrorError commented Jul 5, 2020 •
edited
Loading

@ErrorErrorError is this PR ready to be merged? Do we need to wait for APIs for security and country code?

We can wait for security and country code api since io_station_info does not actually include those information, however what I can do is just hide those two items in the meantime while we wait until the security and country code api is implemented if you'd like. Other than that everything can be merged.

@ErrorErrorError
Copy link
Collaborator Author

ErrorErrorError commented Jul 5, 2020 •
edited
Loading

It's now ready to be merged. Squashed commits and now should be easier to read. Thanks for the help on this pull request!

@ErrorErrorError
Added support to detect if the wifi is connected to a given network.
f5db547 
Added Wi-Fi details when pressing Options and Wi-Fi icon.
- still missing getting ip address, router, internet reachability, security and country code on the connected network
- added disconnect ssid however it does nothing as of now.

Fixed issue with local IP address and router IP address not showing properly
- moved code in Api.c

Added support for reachability to detect if a wireless connection can connect
- added placeholder for wifi channel width
- fixed a typo for Tx Rate

Replaced custom bandwidth to itlwm's api
- Added disassociate button from itlwm's api

Improvements on local address

Improvements getting router ip address
- using scutil instead of netstat and ipconfig, reduces parsing strings

removed unused function runCommand

Removed unnecessary negative sign from rssi and noise due to fixes in itlwm. OpenIntelWireless/itlwm@dd4bec0

Updated rssi and noise values to match itlwm.kext api OpenIntelWireless/itlwm@dd4bec0
- Hidden Security and country code items since they have not been implemented so this pull request can be merged.
@williambj1 williambj1 requested a review from Goshin July 5, 2020 04:07
Goshin
Goshin approved these changes Jul 5, 2020
View reviewed changes
Copy link
Collaborator

@Goshin Goshin left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@Goshin Goshin merged commit 4180f4d into OpenIntelWireless:master Jul 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zxystd zxystd zxystd left review comments

@williambj1 williambj1 williambj1 approved these changes

@Goshin Goshin Goshin approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Milestone
v1.0.0
Development

Successfully merging this pull request may close these issues.

Improvements for StatusMenu
5 participants
@ErrorErrorError @zxystd @williambj1 @igorkulman @Goshin