luci-app-nordvpnlite: update to meet all feature.#1801
Conversation
Changes: - Port the expanded LuCI settings view from the standalone luci-app-nordvpnlite app. - Add service enabled flag handling through UCI. - Add service status and control actions for start, restart, stop, enable and disable. - Add runtime status lookup and display for Telio state, tunnel IP and exit node details. - Add VPN selection modes for recommended server, country code and specific server. - Add country list lookup from the nordvpnlite command. - Add NordVPN server lookup using the public API, hostname normalization and automatic IP/public key fill. - Expand the rpcd ucode backend with the new config, service, lookup and runtime status methods. - Expand the LuCI ACL grants and update the menu ACL dependency for the app. - Add runtime dependencies required by the new LuCI backend: curl, jq and ca-bundle.
|
Hi! Just to let you know, I've just started looking at it. Most likely I'll send you my feedback tomorrow. I've also send the ipk to our QA engineer so he can give his opinion on the UI/UX. Thanks again for the contribution and for your patience |
jjanowsk
left a comment
There was a problem hiding this comment.
Hi! I'll leave my comments free standing because they are mostly UI/UX related. Once we polish that I'll review the code itself.
Major:
- I'm having trouble understanding what is the point of "Enabled" button. Can you please explain that?
- It's a little counter intuitive when you install the app, paste the "Authentication token" and try to start the service for the first time. It will fail, because the token is not yet saved in the config file. It's obvious to us what's happening, but not very clear to the user. It might be hard to figure out that he needs to first click the "Save & Apply" button, and only then he should click the "Start" button. Moreover the app just hangs in that situation.
- Similar situation happens when the user changes configuration e.g. from recommended server to the server from the country list. Changing those settings and clicking restart does not apply the new configuration. That's again counter-intuitive.
- Touching on the server configuration. I'm not sure I'm sold on the idea of selecting "Specific server" via hostname. Right now the nordvpnlite config allows only configuration via pub key and IP address and we could leave it this way in the OpenWRT UI. The problem I see with the current solution is that you need to query ~30MB server list and filter it in memory on the device. It can be slow. Also, by dropping this feature we can get rid of extra dependencies for our app.
- Why is clicking "Disable autostart", blocking "Start", "Restart", "Stop" buttons? It seems to be a bug.
Minor:
- Status texts should not end with ".". E.g. "Stopped" not "Stopped."
- Field names and field values are unaligned, at least in chrome:
Save pending settings before starting or restarting the service and keep runtime controls independent from autostart. Align the embedded init script with the OpenWrt package, simplify specific server configuration to address and public key, and remove the full server-list lookup dependencies.
|
Hi @jjanowsk, The service switch is a runtime master switch stored in /etc/config/nordvpnlite. Unlike autostart, it determines whether the init script is allowed to start the daemon. Consequently, changing the selected country or other settings and clicking Restart now applies the new configuration. I removed hostname-based server lookup. Specific server configuration now accepts the IP address and WireGuard public key directly, matching the native nordvpnlite configuration. The full server-list download and the curl, jq and ca-bundle dependencies were removed. Autostart is now independent from runtime service controls. Disabling autostart no longer blocks Start, Restart or Stop. I also removed trailing periods from status texts and adjusted the status field alignment. Thanks again for the review! |
Dear NordVPN team, as suggest I reopen it and I hope now it will be reviewed by the team.
Extend the NordVPN Lite LuCI backend ACLs and RPC handlers so the settings page can query service state, runtime status, country lists and per-server WireGuard metadata.
Rework the settings view to support recommended, country and specific server selection, fetch country and server data on demand, expose service start/restart/stop/enable/disable controls, and restart the service after Save & Apply while polling for the updated runtime state.
Example:



Changes:
Port the expanded LuCI settings view from the standalone luci-app-nordvpnlite app.
Add service enabled flag handling through UCI.
Add service status and control actions for start, restart, stop, enable and disable.
Add runtime status lookup and display for Telio state, tunnel IP and exit node details.
Add VPN selection modes for recommended server, country code and specific server.
Add country list lookup from the nordvpnlite command.
Add NordVPN server lookup using the public API, hostname normalization and automatic IP/public key fill.
Expand the rpcd ucode backend with the new config, service, lookup and runtime status methods.
Expand the LuCI ACL grants and update the menu ACL dependency for the app.
Add runtime dependencies required by the new LuCI backend: curl, jq and ca-bundle.
Problem
Not so useful Luci app
Solution
After merged is more complete
Run tested on openwrt snapshot