Implementing automatic "safe display mode" for e-ink devices. #17646

Scapesfear · 2024-12-21T08:15:28Z

Purpose / Description

I am solving issue #17618 by implementing the initializeSafeDisplayMode() and hasEInkDisplay() method

Approach

The initializeSafeDisplayMode() method activates safe display mode if the device has an e-ink display and the user hasn't modified this setting. The hasEInkDisplay() method, which detects e-ink displays using a project-specific lookup knownEInkDevices set file of manufacturers and models.

How Has This Been Tested?

I added { "manufacturer": "Google", "models": ["sdk_gphone64_x86_64"] } ( for Pixel 5) and tested it , i have added video of it

Screen_recording_20241222_122258.mp4

Checklist

Please, go through these checks before submitting the PR.

You have a descriptive commit message with a short title (first line, max 50 chars).
You have commented your code, particularly in hard-to-understand areas
You have performed a self-review of your own code
UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
UI Changes: You have tested your change using the Google Accessibility Scanner

welcome · 2024-12-21T08:15:31Z

First PR! 🚀 We sincerely appreciate that you have taken the time to propose a change to AnkiDroid! Please have patience with us as we are all volunteers - we will get to this as soon as possible.

Scapesfear · 2024-12-21T08:17:11Z

now in future commit i will implement hasEInkDisplay() method's complete logic

david-allison · 2024-12-21T09:14:17Z

Please don't submit a PR until it's ready for review. I've marked this as a draft for now, please un-draft when completed

Scapesfear · 2024-12-21T13:24:38Z

sorry for that, will keep this thing in mind for future.

Scapesfear · 2024-12-21T17:41:07Z

I’ve added the hasEInkDisplay() method to detect E-Ink devices and integrated it with initializeSafeDisplayMode() to set preferences. Could you review the implementation and suggest improvements?

Also, since Android Studio lacks E-Ink emulation, how would you recommend testing this functionality?

mikehardy · 2024-12-21T18:13:40Z

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt

+    private fun hasEInkDisplay(): Boolean {
+        return try {
+            // Load JSON from assets
+            val jsonFileName = "eink_devices.json"


is there a real benefit to having this in a JSON file and loading it vs just having a map of manufacturers with an array of their modules in the code? I'm not sure I see a benefit

Externalizing data improves code readability, maintainability, and future-proofing by separating data from logic. This modular approach simplifies updates and supports dynamic configurations(that can be introduced in future ), but a hardcoded map can be implemented if preferred.

as you can see my current json file which contains almost 90-95% e-ink devices is huge , add these many code lines in the code itself just for populating the map seems inefficient , but i can implement map if you want . please review

No need for a file. Hardcode it so we're doing less I/O on startup

the file isn't that big

"locality" for a feature is a value of it's own in my opinion - by which I mean "everything related to a feature in one spot"

Okay i will get it done , i will make an map of eink_devices, but i want to confirm should i make the map in the hasEInkDisplay method itself or another method wich return the map to be used , please clarify

Separate class, it doesn't belong in AnkiDroidApp

I think that could be overkill , to make a seperate DeviceInfo class , making private map of knowin eink devices of key - manufacturer string and value as list of string of models , is it fine ?

mikehardy · 2024-12-21T18:14:15Z

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt

+     * This method checks if the `safe_display` key is already set in the shared preferences.
+     * If the key is not set and the device has an e-ink display, it sets the `safe_display` key to `true`.
+     */
+    @OptIn(UnstableApi::class)


curious, what are the unstable APIs used here and below and why are they necessary vs a stable API ?

I initially used OptIn(UnstableApi::class) due to reliance on experimental APIs like android.system.Os and android.media3.common.util.Log for advanced system operations and logging. However, I've since replaced these unstable APIs with Timber, a stable and widely-used logging tool, ensuring better stability and compatibility while maintaining the required functionality.

i have finally fixed all the timber issues that were present in my code , please review, the issues were , i was not able to see my timber logs with "SafeDisplayMode" tag in the log cat , and when i checked with some Log.d statemnts i came to know my Timber tree was not getting intialized , thus i made a intializeTimberTree method

xenonnn4w · 2024-12-22T03:26:02Z

since Android Studio lacks E-Ink emulation, how would you recommend testing this functionality?

You may try adding something like this
{ "manufacturer": "Google", "models": ["sdk_gphone64_x86_64"] }
to your json file or the array. Also in models change the attribute value to $deviceModel, here it was for Pixel 5 .

david-allison · 2024-12-22T19:54:15Z

AnkiDroid/src/main/assets/eink_devices.json

@@ -1,22 +1,142 @@
 [
  {
-    "manufacturer": "Onyx",
-    "models": ["Boox Note Air", "Boox Note 5", "Boox Nova Air", "Boox Nova 3", "Boox Poke 3", "Boox Tab Ultra"]
+    "manufacturer": "Onyx Boox",


Where did you source this data from?

i did some research used these websites,
https://www.androidauthority.com/best-e-ink-tablet-3230616
https://www.androidpolice.com/best-e-ink-tablets
https://cincodias.elpais.com/smartlife/gadgets/2024-10-23/nuevos-boox-note-air4-c-palma-2-note-max-caracteristicas.html
https://www.androidcentral.com/best-e-ink-tablet

and in the end used Ai tools for completing it

Are you sure that this is what the devices are reporting as manufacturer/model?

sorry i can't confirm 100% about that because nowhere it is directly written what does Build.MODEL print for the devices , although there were some issue in my data i am fixing that .

any guidance on how to collect the accurate data will be extremely helpful

hi , i was asking about keeping a naming convention for models in the map can i keep names such that the names only contains numbers and alphabets that too lowercase , no symbols like "-" or spaces , and then after getting the Build.Manufacturer and Build.Model , i can process them to remove any symbols and sapcing for better mathcing , please suggest ?

No, find a list which you know is accurate, even if it's incomplete, rather than AI-generated slop. If you're not certain, it's best not to include it (and therefore, I believe we're starting from scratch).

My first example found from a quick search in the repo doesn't match. You can bounce from this to a decent source.

Manufacturer = ONYX Model = NovaAir

You'll want to use an authoritative source. None of those blogs are usable

david-allison · 2024-12-22T19:55:27Z

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt

+     * Returns true if a match is found, false otherwise.
+     */
+    fun hasEInkDisplay(): Boolean {
+        initializeTimber("SafeDisplayMode")


Eh? Why not initialize this after logging is setup?

It doesn't need to be done so early

david-allison · 2024-12-22T19:56:04Z

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt

+    private fun hasEInkDisplay(): Boolean {
+        return try {
+            // Load JSON from assets
+            val jsonFileName = "eink_devices.json"


No need for a file. Hardcode it so we're doing less I/O on startup

david-allison · 2024-12-22T19:56:23Z

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt

+            val deviceManufacturer = Build.MANUFACTURER.lowercase().trim()
+            val deviceModel = Build.MODEL.lowercase().trim()
+
+            Timber.tag("SafeDisplayMode").d("Checking device: manufacturer=$deviceManufacturer, model=$deviceModel")


This is a non-idiomatic use of Timber, please fix to match our code style.

david-allison · 2024-12-22T19:58:18Z

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt

+            for (i in 0 until jsonArray.length()) {
+                val deviceInfo = jsonArray.getJSONObject(i)
+                val manufacturer = deviceInfo.getString("manufacturer").lowercase().trim()
+                val models = deviceInfo.getJSONArray("models")
+
+                // If manufacturer matches, check models
+                if (deviceManufacturer.contains(manufacturer) || manufacturer.contains(deviceManufacturer)) {
+                    for (j in 0 until models.length()) {
+                        val model = models.getString(j).lowercase().trim()
+                        if (deviceModel.contains(model) || model.contains(deviceModel)) {
+                            Timber.tag("SafeDisplayMode").d("E-Ink display detected: Manufacturer=$manufacturer, Model=$model")
+                            return true
+                        }
+                    }
+                }
+            }


This method feels like it should be ~1 line of code:

return knownEInkDevices.contains(DeviceInfo(Build.MANUFACTURER, Build.MODEL))

okay , i can get i done , but this requires creating an inner class DeviceInfo and an companion object ( knownEInkDevices map ) is it allowed ?

It's pseudocode, but it feels like a reasonable solution, is there a reason you're asking in partiuclar?

`hasEInkDisplay` method which uses JSON file.

…nfirmed devices,modified `hasEInkDisplay` method.

david-allison

Thanks!

david-allison · 2024-12-27T18:19:19Z

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt

@@ -267,6 +269,191 @@ open class AnkiDroidApp :
        TtsVoicesFieldFilter.ensureApplied()
    }

+    data class DeviceInfo(


Move all this logic into a separate object/class, it doesn't belong here

Done , i have moved all the logic of detecting EInk Device in another new class EInkDeviceIdentifier class which detect if device is EInk or not , only the IntializeSafeDisplay method will remain in the AnkiDroidApp.kt

david-allison · 2024-12-27T18:20:07Z

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt

+
+        if (partialMatch) {
+            // Log potential E-ink device for investigation
+            Timber.w("Potential E-ink device: $currentDevice")


This needs to be sent to ACRA

will it be done like this ?

if (isPartialMatch) {
Timber.w("Potential E-ink device: $currentDevice")
ACRA.errorReporter.handleSilentException(Exception("Potential E-ink device: $currentDevice"))
}

is it correct way of doing it

david-allison · 2024-12-27T18:23:22Z

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt

+                    currentDevice.manufacturer.contains(device.manufacturer.lowercase().trim()) ||
+                        device.manufacturer
+                            .lowercase()
+                            .trim()
+                            .contains(currentDevice.manufacturer)


Why is the lowercasing being done here + is it being done in a locale-invariant fashion? [https://en.wikipedia.org/wiki/Dotless_I]

Should be done on construction, or at the very least using a case-insensitive comparator

david-allison · 2024-12-27T18:23:35Z

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt

+        val exactMatch =
+            knownEInkDevices.any { device ->
+                currentDevice.manufacturer == device.manufacturer.lowercase().trim() &&
+                    currentDevice.model == device.model.lowercase().trim()
+            }


No need to iterate the list twice

david-allison · 2024-12-27T18:23:52Z

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt

+                manufacturer = Build.MANUFACTURER.lowercase().trim(),
+                model = Build.MODEL.lowercase().trim(),
+            )
+        Timber.d("Checking device: $currentDevice")


Suggested change

Timber.d("Checking device: $currentDevice")

Timber.v("Checking device: $currentDevice")

david-allison · 2024-12-27T18:26:28Z

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt

+            Timber.d("E-Ink display detected and safeDisplay is not set. Setting it now.")
+            preferences.edit().putBoolean("safeDisplay", true).apply()
+        } else {
+            Timber.d("No changes made. Either safeDisplay is already set or no confirmed E-Ink display detected.")


Not worth logging

david-allison · 2024-12-27T18:27:14Z

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt

+
+    /**
+     * Checks if the device has an E-Ink display by matching its manufacturer and model.
+     * Returns true if a match is found, false otherwise.


Use @return and wrap true in backticks

Suggested change

* Returns true if a match is found, false otherwise.

* Returns true if a match is found, false otherwise.

david-allison · 2024-12-27T18:27:30Z

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt

+    }
+
+    /**
+     * Checks if the device has an E-Ink display by matching its manufacturer and model.


No need to explain the implementation in the docstring, this should be a method comment

david-allison · 2024-12-27T18:28:56Z

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt

+            DeviceInfo("Onyx", "Go103"),
+            DeviceInfo("Onyx", "Go06"),
+            DeviceInfo("Onyx", "GoColor7"),
+            DeviceInfo("Onyx", "KANT"),
+            DeviceInfo("Onyx", "MC_GULLIVER"),
+            DeviceInfo("Onyx", "Leaf"),
+            DeviceInfo("Onyx", "Leaf2"),
+            DeviceInfo("Onyx", "Leaf3"),
+            DeviceInfo("Onyx", "Leaf3C"),
+            DeviceInfo("Onyx", "LIVINGSTONE"),
+            DeviceInfo("Onyx", "Lomonosov"),
+            DeviceInfo("Onyx", "Max2"),
+            DeviceInfo("Onyx", "Max2Pro"),
+            DeviceInfo("Onyx", "Max3"),
+            DeviceInfo("Onyx", "NoteAir2P"),
+            DeviceInfo("Onyx", "NotePro"),
+            DeviceInfo("Onyx", "NoteS"),
+            DeviceInfo("Onyx", "NoteX"),
+            DeviceInfo("Onyx", "NoteX2"),
+            DeviceInfo("Onyx", "NoteX3"),
+            DeviceInfo("Onyx", "Note_YDT"),
+            DeviceInfo("Onyx", "Nova"),
+            DeviceInfo("Onyx", "Nova2"),
+            DeviceInfo("Onyx", "Nova3"),
+            DeviceInfo("Onyx", "Nova3Color"),
+            DeviceInfo("Onyx", "Nova5"),
+            DeviceInfo("Onyx", "NovaAir"),
+            DeviceInfo("Onyx", "NovaAirC"),
+            DeviceInfo("Onyx", "NovaPro"),
+            DeviceInfo("Onyx", "Tab10CPro"),
+            DeviceInfo("Onyx", "TabMiniC"),
+            DeviceInfo("Onyx", "TabUltra"),
+            DeviceInfo("Onyx", "TabUltraC"),
+            DeviceInfo("Onyx", "TabX"),
+            DeviceInfo("Onyx", "Poke2"),
+            DeviceInfo("Onyx", "Poke3"),
+            DeviceInfo("Onyx", "Poke4"),
+            DeviceInfo("Onyx", "Poke4Lite"),
+            DeviceInfo("Onyx", "Poke5"),
+            DeviceInfo("Onyx", "Poke5P"),
+            DeviceInfo("Onyx", "Page"),
+            DeviceInfo("Onyx", "Palma"),
+            DeviceInfo("Onyx", "SP_NoteSL"),
+            DeviceInfo("Onyx", "SP_PokeL"),
+            DeviceInfo("Onyx", "jdread"),
+            DeviceInfo("boeye", "c64p"),
+            DeviceInfo("boyue", "ares"),
+            DeviceInfo("boyue", "alita"),
+            DeviceInfo("boyue", "p6"),
+            DeviceInfo("boyue", "p61-k12-l"),
+            DeviceInfo("boyue", "p78"),
+            DeviceInfo("boyue", "p101"),
+            DeviceInfo("boyue", "s62"),
+            DeviceInfo("boyue", "t61"),
+            DeviceInfo("boyue", "t62"),
+            DeviceInfo("boyue", "t65s"),
+            DeviceInfo("boyue", "muses"),
+            DeviceInfo("boyue", "mars"),
+            DeviceInfo("boyue", "t80s"),
+            DeviceInfo("boyue", "mimas"),
+            DeviceInfo("crema", "note"),
+            DeviceInfo("crema", "keplerb"),
+            DeviceInfo("crema", "crema-0710c"),
+            DeviceInfo("crema", "crema-0670c"),
+            DeviceInfo("energysistem", "ereader"),
+            DeviceInfo("fidibo", "fidibook"),
+            DeviceInfo("freescale", "evk_6sl_eink"),
+            DeviceInfo("hyread", "k06nu"),
+            DeviceInfo("artatech", "prime"),
+            DeviceInfo("rockchip", "inkpalmplus"),
+            DeviceInfo("rockchip", "moaanmix7"),
+            DeviceInfo("rockchip", "pubook"),
+            DeviceInfo("onyx", "rk30sdk"),
+            DeviceInfo("onyx", "nabukreg_hd"),
+            DeviceInfo("linfiny", "ent-13t1"),
+            DeviceInfo("haoqing", "m6"),
+            DeviceInfo("haoqing", "m7"),
+            DeviceInfo("haoqing", "p6"),
+            DeviceInfo("rockchip", "moaanmix7"),
+            DeviceInfo("allwinner", "mooink plus 2c"),
+            DeviceInfo("barnesandnoble", "bnrv1000"),
+            DeviceInfo("barnesandnoble", "bnrv1100"),
+            DeviceInfo("barnesandnoble", "bnrv1300"),
+            DeviceInfo("barnesandnoble", "bnrv510"),
+            DeviceInfo("barnesandnoble", "bnrv520"),
+            DeviceInfo("barnesandnoble", "bnrv700"),
+            DeviceInfo("onyx", "c67"),
+            DeviceInfo("onyx", "darwin7"),
+            DeviceInfo("onyx", "darwin9"),
+            DeviceInfo("onyx", "edison"),
+            DeviceInfo("onyx", "faust3"),
+            DeviceInfo("onyx", "kon_tiki2"),
+            DeviceInfo("onyx", "poke_pro"),
+            DeviceInfo("onyx", "tabultra"),
+            DeviceInfo("rockchip", "pubook"),
+            DeviceInfo("ridi", "ridipaper"),
+            DeviceInfo("sony", "dpt-cp1"),
+            DeviceInfo("sony", "dpt-rp1"),
+            DeviceInfo("onyx", "tagus_pokep"),
+            DeviceInfo("xiaomi", "xiaomi_reader"),


This doesn't distinguish between "known good" and "maybe"

This doesn't include the sample "known good" device from the original issue (Viwoods)

This doesn't include sources for the list

This has onyx and Onyx, why?

onyx and Onyx didn't really made difference according to my code , as i was just comparing the invariants , but i will change that , can i store everything in lowercase ?

Store the known good values as they are on the device. Perform a case-insensitive, locale invariant comparison in the code

In the code files, the manufacturer was listed as onyx. However, the CSV file didn’t specify a manufacturer, only the models . Since I saw NovaAir in the CSV and you provided an example with Onyx I assumed the manufacturer to be Onyx for any unclear entries. Let me know if any adjustments are needed.

To simplify things:

We have a list of known good data.

We have a few lists of 'probably good data'

Each category needs a source

We have a few manufacturers who probably make e-ink devices

Our goal is to populate the list of known good data.

We do this via a silent troubleshooting report from users who have opted in.

====

If current data is a known good, it's an e-ink

If current data matches a 'probably', assume it's an e-ink and report so we have accurate data

If current data matches manufacturer only, report and do nothing

david-allison · 2024-12-27T18:29:58Z

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt

@@ -144,6 +144,8 @@ open class AnkiDroidApp :
        Timber.d("Startup - Application Start")
        Timber.i("Timber config: $logType")

+        // Initialize safe display mode for E-ink devices
+        initializeSafeDisplayMode()


This shouldn't be done before crash handling etc...

…wnEinkDevices along with source of data

david-allison · 2025-01-03T23:09:10Z

AnkiDroid/src/main/java/com/ichi2/anki/EInkDeviceIdentifier.kt

+import java.util.Locale
+
+class EInkDeviceIdentifier {
+    data class DeviceInfo(


Data class doesn't feel useful here: we're not using exact comparators

I really think we should keep DeviceInfo as a data class. It’s all about storing data in a clean and simple way, which makes sense since it's just holding the device's information. Mixing it into EInkDeviceIdentifier feels like it’s overcomplicating things and doesn’t really follow the separation of concerns principle.

Also, looking ahead, I see EInkDeviceIdentifier evolving with more features as e-ink devices continue to grow in popularity. These devices are more energy-efficient, and as research progresses, we’ll likely see more of them in the future. Keeping things separate now will help us easily scale and add more functionality later on.

david-allison · 2025-01-03T23:09:52Z

AnkiDroid/src/main/java/com/ichi2/anki/EInkDeviceIdentifier.kt

+    // Checks if the device has an E-Ink display by matching its manufacturer and model.
+    fun isEInkDevice(): Boolean {
+        val currentDevice = DeviceInfo.current
+        Timber.v("Checking device: $currentDevice")


Timber prefers %s

i will fix that

david-allison · 2025-01-03T23:12:20Z

AnkiDroid/src/main/java/com/ichi2/anki/EInkDeviceIdentifier.kt

+            return true
+        }
+        // If the device is a partial match or if the manufacturer is in the list of known E-ink manufacturers then report it
+        if (isPartialMatch || eInkManufacturersList.contains(currentDevice.manufacturer)) {


Split out obtaining the output, and performing actions based on the outputs

both the manufaturer match and partial match results in repoting the device only so why should i seperate them

david-allison · 2025-01-03T23:19:18Z

AnkiDroid/src/main/java/com/ichi2/anki/EInkDeviceIdentifier.kt

@@ -17,8 +17,9 @@ class EInkDeviceIdentifier {
            get() = originalModel.lowercase(Locale.ROOT).trim()


These shouldn't recompute the value each time if the function is pure

yeah you are right , i will fix that

david-allison · 2025-01-03T23:33:15Z

AnkiDroid/src/main/java/com/ichi2/anki/EInkDeviceIdentifier.kt

@@ -185,14 +182,16 @@ class EInkDeviceIdentifier {
        // Check if the device is an exact match or a partial match.
        for (device in knownEInkDevices) {
            // Check if the device is an exact match.
-            if (currentDevice.manufacturer == device.manufacturer &&
+            if (
+                currentDevice.manufacturer == device.manufacturer &&
                currentDevice.model == device.model
            ) {
                isExactMatch = true
                break
            }
            // Check if the device is a partial match. Partial matches are detected using substring matching.


? partial matches are defined as a match on either manufacturer [or casing issues]

I am reporting potential e-ink if there is a manufacturer match or partial match , the partial match algorithm is that it checks if the current device's manufaturer and model is substring of any of the pair of manufaturer and model in the knowEInkDevices set or vice-versa as in the below source
you can see that there is not a great certainity of perfect match but they are checking a substring match https://github.com/koreader/android-luajit-launcher/blob/6bba3f4bb4da8073d0f4ea4f270828c8603aa54d/app/src/main/java/org/koreader/launcher/device/DeviceInfo.kt#L260

few more sources doing the same

https://github.com/plotn/coolreader/blob/e5baf0607e678468aa045053ba5f092164aa1dd7/android/src/org/coolreader/crengine/DeviceInfo.java#L201

https://github.com/plotn/coolreader/blob/e5baf0607e678468aa045053ba5f092164aa1dd7/android/src/org/coolreader/crengine/DeviceInfo.java#L216

david-allison · 2025-01-03T23:38:37Z

AnkiDroid/src/main/java/com/ichi2/anki/EInkDeviceIdentifier.kt

+        var isExactMatch = false
+        var isPartialMatch = false
+
+        // Check if the device is an exact match or a partial match.
+        for (device in knownEInkDevices) {


The code + loop feels too complicated:

Split out getting the match type, and executing based on the match

Then simplify the code querying the match type

I understand the suggestion of splitting out the match type, but this would require iterating through the list twice, which introduces a tradeoff between clarity and efficiency. The exact match can be checked using ==, while the partial match requires a substring comparison. These two comparisons are fundamentally different and cannot be handled in a single pass without mixing their logic. Initially, I separated the checks, but you mentioned not to iterate the list twice, which is why I combined the logic.

david-allison · 2025-01-03T23:39:09Z

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt

+    private fun initializeSafeDisplayMode() {
+        val preferences = this.sharedPrefs()
+        val isSafeDisplaySet = preferences.contains("safeDisplay")
+        val deviceIdentifier = EInkDeviceIdentifier()


This likely doesn't need to be a class if you're calling a method on it and it has no state

I’m having difficulty understanding the point you’re trying to convey. Could you clarify or elaborate further?

david-allison · 2025-01-03T23:39:42Z

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt

@@ -265,6 +265,23 @@ open class AnkiDroidApp :
        TtsVoices.launchBuildLocalesJob()
        // enable {{tts-voices:}} field filter
        TtsVoicesFieldFilter.ensureApplied()
+        // initialize safe display mode for e-ink devices
+        initializeSafeDisplayMode()


performance: this should be done in the background if at all possible so it doesn't block startup

I'm unsure how to handle this perfectly, but the safe display mode must be initialized at app startup if the device is e-ink. This won't block startup since it only checks for e-ink and initializes safe mode if needed, without interfering with other processes.

david-allison · 2025-01-03T23:40:08Z

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt

+        val isSafeDisplaySet = preferences.contains("safeDisplay")
+        val deviceIdentifier = EInkDeviceIdentifier()
+        if (!isSafeDisplaySet && deviceIdentifier.isEInkDevice()) {
+            preferences.edit().putBoolean("safeDisplay", true).apply()


there is an .edit { } extension method which implicitly calls apply

david-allison · 2025-01-03T23:41:39Z

AnkiDroid/src/main/java/com/ichi2/anki/EInkDeviceIdentifier.kt

+        // If the device is a partial match or if the manufacturer is in the list of known E-ink manufacturers then report it
+        if (isPartialMatch || eInkManufacturersList.contains(currentDevice.manufacturer)) {
+            Timber.w("Potential E-ink device: $currentDevice")
+            ACRA.errorReporter.handleSilentException(Exception("Potential E-ink device: $currentDevice"))


I added better diagnostics in: #17686

We should extract this and also use it here, so we get a useful fingerprint

david-allison · 2025-01-04T13:59:44Z

This feature is not expected to add much value to the app based on proportion of e-ink users as a percentage of the total userbase and we have to be very careful with our time which is always our biggest limitation.

This PR doesn't provide enough value to justify the time spent already, much less any future time.

With apologies, I'm closing this as I'm not considering spending any further time on it

Scapesfear · 2025-01-04T15:37:54Z

This feature is not expected to add much value to the app based on proportion of e-ink users as a percentage of the total userbase and we have to be very careful with our time which is always our biggest limitation.

This PR doesn't provide enough value to justify the time spent already, much less any future time.

With apologies, I'm closing this as I'm not considering spending any further time on it

Hey , I’ve implemented nearly all the changes you asked for, but I got frustrated with the lint issues since I wasn’t familiar with them. I’ve invested a lot of time into this.

If possible, could you please reconsider reopening the PR?

mikehardy · 2025-01-04T19:23:59Z

I’ve implemented nearly all the changes you asked for,

There is no reasonable expectation that future costs to review + get this merged will be sufficient to justify further activity, based on past activity.

Thus this statement is accurate as a standalone:

This feature is not expected to add much value to the app based on proportion of e-ink users as a percentage of the total userbase and we have to be very careful with our time which is always our biggest limitation

This PR doesn't provide enough value to justify the time spent already, much less any future time.

This following statement is a logical fallacy when allocating time + resources, and though it feels natural and is understandable after any investment, it should not be used as a justification:

I’ve invested a lot of time into this

https://en.wikipedia.org/wiki/Sunk_cost

github-actions bot added the New contributor label Dec 21, 2024

david-allison marked this pull request as draft December 21, 2024 09:14

david-allison added the Needs Author Reply Waiting for a reply from the original author label Dec 21, 2024

Scapesfear marked this pull request as ready for review December 21, 2024 17:34

mikehardy reviewed Dec 21, 2024

View reviewed changes

Scapesfear marked this pull request as draft December 22, 2024 07:11

Scapesfear marked this pull request as ready for review December 22, 2024 09:08

david-allison requested changes Dec 22, 2024

View reviewed changes

Scapesfear marked this pull request as draft December 23, 2024 06:04

Scapesfear marked this pull request as ready for review December 24, 2024 04:34

Scapesfear marked this pull request as draft December 24, 2024 06:06

Scapesfear marked this pull request as ready for review December 24, 2024 06:39

Implementing automatic "safe display mode" for e-ink devices and basic

e6efbdc

`hasEInkDisplay` method which uses JSON file.

Scapesfear force-pushed the safeDisplay1 branch 2 times, most recently from a6635ff to 64f04e5 Compare December 24, 2024 11:56

Removed eink_devices.json , made knownEInkDevices set with list of co…

d7a3cf9

…nfirmed devices,modified `hasEInkDisplay` method.

Scapesfear force-pushed the safeDisplay1 branch from 95b7dae to d7a3cf9 Compare December 27, 2024 07:56

Scapesfear requested a review from david-allison December 27, 2024 08:10

david-allison requested changes Dec 27, 2024

View reviewed changes

Scapesfear added 4 commits January 3, 2025 17:02

moved EInkDetection to new class EInkDeviceIdentifier,populated kno…

8967aec

…wnEinkDevices along with source of data

TRY

fc29239

TRY 2

87a54de

TRY 3

2f4fbf7

david-allison requested changes Jan 3, 2025

View reviewed changes

Scapesfear added 8 commits January 4, 2025 12:33

Merge branch 'refs/heads/main' into safeDisplay1

b991ef2

simplifying code

0836814

simplifying code

4ce520c

simplifying code

36f6448

TRY 6

2ea33e0

ohh my god how do i even fix the lint issues 😭

8ae2dd0

ohh my god how do i even fix the lint issues 😭 2

9fb9fd0

ohh my god how do i even fix the lint issues 😭 3

b384c00

david-allison closed this Jan 4, 2025

	Timber.d("Checking device: $currentDevice")
	Timber.v("Checking device: $currentDevice")

	* Returns true if a match is found, false otherwise.
	* Returns true if a match is found, false otherwise.

		@@ -17,8 +17,9 @@ class EInkDeviceIdentifier {
		get() = originalModel.lowercase(Locale.ROOT).trim()

Implementing automatic "safe display mode" for e-ink devices. #17646

Implementing automatic "safe display mode" for e-ink devices. #17646

Conversation

Scapesfear commented Dec 21, 2024 • edited Loading

Purpose / Description

Approach

How Has This Been Tested?

Checklist

welcome bot commented Dec 21, 2024

Scapesfear commented Dec 21, 2024

david-allison commented Dec 21, 2024

Scapesfear commented Dec 21, 2024

Scapesfear commented Dec 21, 2024

Choose a reason for hiding this comment

Scapesfear Dec 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Scapesfear Dec 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Scapesfear Dec 22, 2024 • edited Loading

Choose a reason for hiding this comment

Scapesfear Dec 22, 2024 • edited Loading

Choose a reason for hiding this comment

xenonnn4w commented Dec 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Scapesfear Dec 23, 2024 • edited Loading

Choose a reason for hiding this comment

david-allison Dec 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

david-allison left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Scapesfear Dec 29, 2024 • edited Loading

Choose a reason for hiding this comment

david-allison Dec 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Scapesfear commented Dec 21, 2024 •

edited

Loading

Scapesfear Dec 22, 2024 •

edited

Loading

Scapesfear Dec 23, 2024 •

edited

Loading

Scapesfear Dec 22, 2024 •

edited

Loading

Scapesfear Dec 22, 2024 •

edited

Loading

xenonnn4w commented Dec 22, 2024 •

edited

Loading

Scapesfear Dec 23, 2024 •

edited

Loading

david-allison Dec 23, 2024 •

edited

Loading

Scapesfear Dec 29, 2024 •

edited

Loading

david-allison Dec 29, 2024 •

edited

Loading

Scapesfear commented Jan 4, 2025 •

edited

Loading

mikehardy commented Jan 4, 2025 •

edited

Loading