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

display network settings based on device capabilities #4550

Merged
merged 3 commits into from Feb 18, 2019

Conversation

Projects
None yet
3 participants
@pazos
Copy link
Contributor

pazos commented Feb 5, 2019

fixes #4527
fix for android network info, requires koreader/android-luajit-launcher#104

Needs review, @NiLuJe for Kindle @v01d for sony-prstux.

if Device.retrieveNetworkInfo then
if Device:isAndroid() then
UIManager:show(InfoMessage:new{
text = Device:networkInfo(),

This comment has been minimized.

@Frenzie

Frenzie Feb 5, 2019

Member

I think you forgot something?

@@ -90,6 +90,10 @@ function Device:initNetworkManager(NetworkMgr)
end
end

function Device:networkInfo()
return android.getNetworkInfo()

This comment has been minimized.

@Frenzie

Frenzie Feb 5, 2019

Member

Hehe, this. But anyway, like I said there I think this is where any potential string formatting should occur.

This comment has been minimized.

@Frenzie

Frenzie Feb 5, 2019

Member

And that'd probably make it the Android version of retrieveNetworkInfo().

@NiLuJe

This comment has been minimized.

Copy link
Member

NiLuJe commented Feb 5, 2019

Looks harmless enough on the Kindle front, 👍 ;).

if ip == 0 or gw == 0 then
return _("Not connected")
else
return _(string.format("Conected to %s\n ip address: %s\n gateway: %s", ssid, ip, gw))

This comment has been minimized.

@Frenzie

Frenzie Feb 6, 2019

Member

Please use util.format instead.

PS Connected & IP.

This comment has been minimized.

@pazos

pazos Feb 6, 2019

Author Contributor

I can't find util.format.

This comment has been minimized.

@Frenzie

Frenzie Feb 6, 2019

Member

Sorry, I messed up in between don't use string.format and please use util.template. 😊

local T = require("ffi/util").template
        return T(_("Connected to %1\n IP address: %2\n gateway: %3"), ssid, ip, gw)

string.format doesn't allow for free positioning of the provided strings/numbers, which is generally (but not always!) okay between Indo-European languages, but otherwise bad for localization.

@@ -90,6 +91,15 @@ function Device:initNetworkManager(NetworkMgr)
end
end

function Device:retrieveNetworkInfo()
local ssid, ip, gw = string.match(android.getNetworkInfo(), "(.*);(.*);(.*)")

This comment has been minimized.

@Frenzie

Frenzie Feb 6, 2019

Member

Is there a reason not to put this in luajit-launcher?

This comment has been minimized.

@pazos

pazos Feb 6, 2019

Author Contributor

in assets/android?. I don't know, it seems easy to expand here. Plus I'm not to excited messing with JNI.

This comment has been minimized.

@Frenzie

Frenzie Feb 6, 2019

Member

I'm not talking about JNI (that would be, e.g., making an array into a Lua table) but about not taking this backend logic to front.

This comment has been minimized.

@Frenzie

Frenzie Feb 6, 2019

Member

Basically I think that most forward-thinking, front here should just receive a table, grab android.getNetworkInfo().ssid, ip, and gw.

Since it's unlikely that we want more, it could also be okay to just return those three things. But it doesn't seem the best design to put that logic here.

@pazos pazos force-pushed the pazos:network_settings branch 2 times, most recently from 2cb8add to 5c2da95 Feb 6, 2019

@Frenzie Frenzie referenced this pull request Feb 6, 2019

Merged

Refactor #104

@pazos

This comment has been minimized.

Copy link
Contributor Author

pazos commented Feb 7, 2019

Any idea about why on android menus don't get updates after pressing turnOn/turnOff wifi?.

@Frenzie

This comment has been minimized.

Copy link
Member

Frenzie commented Feb 7, 2019

Maybe it's just whatever status Android returned upon first loading the menu?

@pazos

This comment has been minimized.

Copy link
Contributor Author

pazos commented Feb 8, 2019

Maybe it's just whatever status Android returned upon first loading the menu?

Yes. In other devices the status gets updated somehow via complete_callback, but I'm unable to do the same on Android.

Since this "bug" was present before this PR it shouldn't matter for merging this but it will be awesome to fix that too.

@pazos pazos force-pushed the pazos:network_settings branch from cfc5b5f to 5348857 Feb 17, 2019

@pazos

This comment has been minimized.

Copy link
Contributor Author

pazos commented Feb 17, 2019

Ok, it was easy to update menus.

@Frenzie Frenzie merged commit b6683b7 into koreader:master Feb 18, 2019

1 check passed

ci/circleci Your tests passed on CircleCI!
Details

@tob1az tob1az referenced this pull request Mar 8, 2019

Open

Inkpad3 Wifi Standby #4747

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.