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.

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 from 02be2fe to a6635ff Compare December 24, 2024 11:52

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

64f04e5

…nfirmed devices,modified `hasEInkDisplay` method.

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

Scapesfear commented Dec 21, 2024 •

edited

Loading

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

mikehardy Dec 21, 2024

Scapesfear Dec 22, 2024 •

edited

Loading

Scapesfear Dec 22, 2024

david-allison Dec 22, 2024

mikehardy Dec 22, 2024

Scapesfear Dec 23, 2024

david-allison Dec 23, 2024

Scapesfear Dec 23, 2024 •

edited

Loading

mikehardy Dec 21, 2024

Scapesfear Dec 22, 2024 •

edited

Loading

Scapesfear Dec 22, 2024 •

edited

Loading

xenonnn4w commented Dec 22, 2024 •

edited

Loading

david-allison Dec 22, 2024

Scapesfear Dec 23, 2024

david-allison Dec 23, 2024

Scapesfear Dec 23, 2024

Scapesfear Dec 23, 2024

Scapesfear Dec 23, 2024 •

edited

Loading

david-allison Dec 23, 2024 •

edited

Loading

david-allison Dec 22, 2024

david-allison Dec 22, 2024

david-allison Dec 22, 2024

david-allison Dec 22, 2024

Scapesfear Dec 23, 2024

david-allison Dec 23, 2024

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

Are you sure you want to change the base?

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

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