Another round of Kobo Fixes (#3939)

* Trim unneeded stuff from startup script

I was somehow convinced I'd already done that...
While we're there, explain why we need to siphon those specific vars

* Fix a stray eth0

-> $INTERFACE

* Be very very sure we have INTERFACE set in our env

re #3936

* Make getFirmwareVersion less fragile on Kobo

Not that we actually use it right now, but, still. :D

* Use the same syntax as the PRODUCT check

* Actually implement getProductId

Instead of a stray c/p ^^

* Properly identify the Rev2/Mark7 variants of existing devices

Namely, the H2O² and Aura SE
Not that the H2O²r2 support is still broken, this just allows us to
implement it cleanyl without breaking handling of the original H2O²

re #3925

* Tweak sleeps a bit around Kobo WiFi modules...

See if that jog things up (re #3936)

* Try harder not to suspend with WiFi on on Kobos

Because otherwise, things go boom. (re #3936)

frontend/device/generic/device.lua

@@ -142,7 +142,12 @@ function Device:onPowerEvent(ev)
         self.screen_saver_mode = true
         UIManager:scheduleIn(0.1, function()
           local network_manager = require("ui/network/manager")
-          if network_manager.wifi_was_on then
+          -- NOTE: wifi_was_on does not necessarily mean that WiFi is *currently* on! It means *we* enabled it.
+          --       This is critical on Kobos (c.f., #3936), where it might still be on from KSM or Nickel,
+          --       without us being aware of it (i.e., wifi_was_on still unset or false),
+          --       because suspend will at best fail, and at worst deadlock the system if WiFi is on,
+          --       regardless of who enabled it!
+          if network_manager.wifi_was_on or network_manager:isWifiOn() then
               network_manager:releaseIP()
               network_manager:turnOffWifi()
           end

frontend/device/kobo/device.lua

@@ -10,8 +10,10 @@ local function no() return false end
 
 local function koboEnableWifi(toggle)
     if toggle == 1 then
+        logger.info("Kobo WiFi: enabling WiFi")
         os.execute("./enable-wifi.sh")
     else
+        logger.info("Kobo WiFi: disabling WiFi")
         os.execute("./disable-wifi.sh")
     end
 end
@@ -119,6 +121,24 @@ local KoboSnow = Kobo:new{
     },
 }
 
+-- Kobo Aura H2O2, Rev2:
+-- FIXME: This needs fixing, at the very least on the touch protocol front, c.f., #3925
+local KoboSnowRev2 = Kobo:new{
+    model = "Kobo_snow",
+    hasFrontlight = yes,
+    touch_probe_ev_epoch_time = true,
+    touch_phoenix_protocol = true,
+    display_dpi = 265,
+    -- the bezel covers the top 11 pixels:
+    viewport = Geom:new{x=0, y=11, w=1080, h=1429},
+    hasNaturalLight = yes,
+    frontlight_settings = {
+        frontlight_white = "/sys/class/backlight/lm3630a_ledb",
+        frontlight_red = "/sys/class/backlight/lm3630a_led",
+        frontlight_green = "/sys/class/backlight/lm3630a_leda",
+    },
+}
+
 -- Kobo Aura second edition:
 local KoboStar = Kobo:new{
     model = "Kobo_star",
@@ -130,6 +150,18 @@ local KoboStar = Kobo:new{
     viewport = Geom:new{x=1, y=0, w=756, h=1024},
 }
 
+-- Kobo Aura second edition, Rev 2:
+-- FIXME: Confirm that this is accurate? If it is, and matches the Rev1, ditch the special casing.
+local KoboStarRev2 = Kobo:new{
+    model = "Kobo_star",
+    hasFrontlight = yes,
+    touch_probe_ev_epoch_time = true,
+    touch_phoenix_protocol = true,
+    display_dpi = 212,
+    -- the bezel covers 1-2 pixels on each side:
+    viewport = Geom:new{x=1, y=0, w=756, h=1024},
+}
+
 -- Kobo Glo HD:
 local KoboAlyssum = Kobo:new{
     model = "Kobo_alyssum",
@@ -236,8 +268,12 @@ function Kobo:initNetworkManager(NetworkMgr)
         self:showNetworkMenu(complete_callback)
     end
 
+    local net_if = os.getenv("INTERFACE")
+    if not net_if then
+        net_if = "eth0"
+    end
     NetworkMgr:setWirelessBackend(
-        "wpa_supplicant", {ctrl_interface = "/var/run/wpa_supplicant/" .. os.getenv("INTERFACE")})
+        "wpa_supplicant", {ctrl_interface = "/var/run/wpa_supplicant/" .. net_if})
 
     function NetworkMgr:obtainIP()
         os.execute("./obtain-ip.sh")
@@ -250,6 +286,12 @@ function Kobo:initNetworkManager(NetworkMgr)
     function NetworkMgr:restoreWifiAsync()
         os.execute("./restore-wifi-async.sh")
     end
+
+    -- NOTE: Cheap-ass way of checking if WiFi seems to be enabled...
+    --       Since the crux of the issues lies in race-y module unloading, this is perfectly fine for our usage.
+    function NetworkMgr:isWifiOn()
+        return 0 == os.execute("lsmod | grep -q sdio_wifi_pwr")
+    end
 end
 
 function Kobo:supportsScreensaver() return true end
@@ -333,8 +375,37 @@ end
 
 function Kobo:getFirmwareVersion()
     local version_file = io.open("/mnt/onboard/.kobo/version", "r")
-    self.firmware_rev = string.sub(version_file:read(),24,28)
+    if not version_file then
+        self.firmware_rev = "none"
+    end
+    local version_str = version_file:read()
     version_file:close()
+
+    local i = 0
+    for field in util.gsplit(version_str, ",", false, false) do
+        i = i + 1
+        if (i == 3) then
+             self.firmware_rev = field
+        end
+    end
+end
+
+local function getProductId()
+    -- Try to get it from the env first (KSM only)
+    local product_id = os.getenv("PRODUCT_ID")
+    -- If that fails, devise it ourselves
+    if not product_id then
+        local version_file = io.open("/mnt/onboard/.kobo/version", "r")
+        if not version_file then
+            return "000"
+        end
+        local version_str = version_file:read()
+        version_file:close()
+
+        product_id = string.sub(version_str, -3, -1)
+    end
+
+    return product_id
 end
 
 local unexpected_wakeup_count = 0
@@ -540,6 +611,7 @@ end
 -------------- device probe ------------
 
 local codename = Kobo:getCodeName()
+local product_id = getProductId()
 
 if codename == "dahlia" then
     return KoboDahlia
@@ -557,10 +629,14 @@ elseif codename == "alyssum" then
     return KoboAlyssum
 elseif codename == "pika" then
     return KoboPika
+elseif codename == "star" and product_id == "379" then
+    return KoboStarRev2
 elseif codename == "star" then
     return KoboStar
 elseif codename == "daylight" then
     return KoboDaylight
+elseif codename == "snow" and product_id == "378" then
+    return KoboSnowRev2
 elseif codename == "snow" then
     return KoboSnow
 else

platform/kobo/disable-wifi.sh

@@ -4,16 +4,16 @@
 
 killall udhcpc default.script wpa_supplicant 2>/dev/null
 
-[ "${WIFI_MODULE}" != "8189fs" ] && wlarm_le -i eth0 down
+[ "${WIFI_MODULE}" != "8189fs" ] && wlarm_le -i "${INTERFACE}" down
 ifconfig "${INTERFACE}" down
 
 # Some sleep in between may avoid system getting hung
 # (we test if a module is actually loaded to avoid unneeded sleeps)
 if lsmod | grep -q "${WIFI_MODULE}"; then
-    usleep 200000
-    rmmod -r "${WIFI_MODULE}"
+    usleep 250000
+    modprobe -r "${WIFI_MODULE}"
 fi
 if lsmod | grep -q sdio_wifi_pwr; then
-    usleep 200000
-    rmmod -r sdio_wifi_pwr
+    usleep 250000
+    modprobe -r sdio_wifi_pwr
 fi

platform/kobo/enable-wifi.sh

@@ -3,6 +3,8 @@
 # Load wifi modules and enable wifi.
 
 lsmod | grep -q sdio_wifi_pwr || insmod "/drivers/${PLATFORM}/wifi/sdio_wifi_pwr.ko"
+# Moar sleep!
+usleep 250000
 # WIFI_MODULE_PATH = /drivers/$PLATFORM/wifi/$WIFI_MODULE.ko
 lsmod | grep -q "${WIFI_MODULE}" || insmod "${WIFI_MODULE_PATH}"
 # Race-y as hell, don't try to optimize this!

platform/kobo/koreader.sh

@@ -47,22 +47,14 @@ if [ "${FROM_NICKEL}" = "true" ]; then
         fi
     fi
 
-    if [ "${FROM_KFMON}" = "true" ]; then
-        # Siphon nickel's full environment, since KFMon inherits such a minimal one, and that apparently confuses the hell out of Nickel for some reason if we decide to restart it without a reboot...
-        for env in $(xargs -n 1 -0 <"/proc/$(pidof nickel)/environ"); do
-            # shellcheck disable=SC2163
-            export "${env}"
-        done
-    else
-        # Siphon a few things from nickel's env...
-        eval "$(xargs -n 1 -0 <"/proc/$(pidof nickel)/environ" | grep -e DBUS_SESSION_BUS_ADDRESS -e NICKEL_HOME -e WIFI_MODULE -e LANG -e WIFI_MODULE_PATH -e INTERFACE 2>/dev/null)"
-        export DBUS_SESSION_BUS_ADDRESS NICKEL_HOME WIFI_MODULE LANG WIFI_MODULE_PATH INTERFACE
-    fi
+    # Siphon a few things from nickel's env (namely, stuff exported by rcS *after* on-animator.sh has been launched)...
+    eval "$(xargs -n 1 -0 <"/proc/$(pidof nickel)/environ" | grep -e DBUS_SESSION_BUS_ADDRESS -e NICKEL_HOME -e WIFI_MODULE -e LANG -e WIFI_MODULE_PATH -e INTERFACE 2>/dev/null)"
+    export DBUS_SESSION_BUS_ADDRESS NICKEL_HOME WIFI_MODULE LANG WIFI_MODULE_PATH INTERFACE
 
     # flush disks, might help avoid trashing nickel's DB...
     sync
     # stop kobo software because it's running
-    # NOTE: We don't need to kill KFMon, it's smart enough not to allow running concurrent instances of ourselves
+    # NOTE: We don't need to kill KFMon, it's smart enough not to allow running anything else while we're up
     killall nickel hindenburg sickel fickel fmon 2>/dev/null
 fi
 
@@ -98,6 +90,13 @@ if [ ! -n "${PLATFORM}" ]; then
     fi
     export PLATFORM
 fi
+
+# Make sure we have a sane-ish INTERFACE env var set...
+if [ ! -n "${INTERFACE}" ]; then
+    # That's what we used to hardcode anyway
+    INTERFACE="eth0"
+    export INTERFACE
+fi
 # end of value check of PLATFORM
 
 # Remount the SD card RW if it's inserted and currently RO

platform/kobo/nickel.sh

@@ -10,6 +10,7 @@ PATH="/sbin:/bin:/usr/sbin:/usr/bin:/usr/lib:"
 
 # We don't need to duplicate any of the env setup from rcS, since we will only ever run this to *restart* nickel, and not bootstrap it.
 # Meaning we've already got most of the necessary env from nickel itself via both our launcher (fmon/KFMon) and our own startup script.
+# NOTE: LD_LIBRARY_PATH is the only late export from rcS we don't siphon in koreader.sh, for obvious reasons ;).
 export LD_LIBRARY_PATH="/usr/local/Kobo"
 
 # Make sure we kill the WiFi first, because nickel apparently doesn't like it if it's up... (cf. #1520)
@@ -18,12 +19,12 @@ if lsmod | grep -q sdio_wifi_pwr; then
     killall udhcpc default.script wpa_supplicant 2>/dev/null
     [ "${WIFI_MODULE}" != "8189fs" ] && wlarm_le -i "${INTERFACE}" down
     ifconfig "${INTERFACE}" down
-    # NOTE: Kobo's busybox build is weird. rmmod appears to be modprobe in disguise, defaulting to the -r flag. If re-specifying -r starts to fail one day, switch to rmmod without args, or modprobe -r.
+    # NOTE: Kobo's busybox build is weird. rmmod appears to be modprobe in disguise, defaulting to the -r flag. Use modprobe -r just to be safe...
     # c.f., #2394?
-    usleep 200000
-    rmmod -r "${WIFI_MODULE}"
-    usleep 200000
-    rmmod -r sdio_wifi_pwr
+    usleep 250000
+    modprobe -r "${WIFI_MODULE}"
+    usleep 250000
+    modprobe -r sdio_wifi_pwr
 fi
 
 # Flush buffers to disk, who knows.

spec/unit/device_spec.lua

@@ -71,7 +71,7 @@ describe("device module", function()
                 if key == "PRODUCT" then
                     return "trilogy"
                 else
-                    return saved_getenv(key)
+                    return osgetenv(key)
                 end
             end)
 
@@ -118,7 +118,7 @@ describe("device module", function()
                 if key == "PRODUCT" then
                     return "trilogy"
                 else
-                    return saved_getenv(key)
+                    return osgetenv(key)
                 end
             end)
             local kobo_dev = require("device/kobo/device")