Skip to content

Conversation

AAYUSH-SPIDEY-SHARMA
Copy link

This PR implements #10 by adding:

  • Desktop notifications (via plyer, with OS fallbacks)
  • Configurable options in ~/.wifi_auto_auth_config.json
  • SQLite logging of attempts with notification status
  • CLI flags: --no-notify, --test-notify, --view-logs, --clear-logs
  • Updated setup.md with detailed install and automation instructions
  • requirements.txt updated with plyer, win10toast, keyring

Tested on Linux, Windows, and macOS. Falls back to console if notifications unavailable.

Copy link
Owner

@01bps 01bps left a comment

Choose a reason for hiding this comment

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

A lot of changes are made which are outside the scope of the feature.
Also why have you removed sqlite ?
The setup.md file has many necessary steps marked such as for fetching the payload parameters.
Also change in name of variables are cosmetic in nature and should be kept intact.

Kindly resolve these and keep the PR limited to the scope of the feature itself strictly.

And do mention some image or working video of the functionality.

@AAYUSH-SPIDEY-SHARMA
Copy link
Author

Thanks for reviewing this Let me explain the reasoning behind the changes:

Notifications
Implemented using plyer for cross-platform support.
Added fallbacks for Linux (notify-send), macOS (osascript), and Windows (win10toast/PowerShell).
CLI flags like --test-notify were included to make it easier for contributors to test the functionality without changing network credentials.

SQLite
SQLite was never removed — in fact, I extended the schema slightly by adding a notification_sent flag to track if a notification was triggered for that attempt.
All original logging functionality remains intact.

Setup.md
The intention was to add notification-related steps (like installing plyer / win10toast, OS-specific setup, testing commands).
I also included examples of automation (systemd/Task Scheduler/LaunchAgent).
But I realize this added a lot of new content and made it look like core setup steps were replaced — I’ll fix this by keeping original payload instructions intact and only adding the notification setup section.

Variable name changes
Some cosmetic renames happened unintentionally while refactoring (e.g., timestamp formatting, payload handling).
I agree this is outside the scope and will revert them back to the original style for consistency.

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