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

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

Closed
wants to merge 14 commits into from
27 changes: 15 additions & 12 deletions AnkiDroid/src/main/java/com/ichi2/anki/EInkDeviceIdentifier.kt
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ class EInkDeviceIdentifier {
get() = originalModel.lowercase(Locale.ROOT).trim()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah you are right , i will fix that


companion object {
val current: DeviceInfo
get() = DeviceInfo(Build.MANUFACTURER, Build.MODEL)
val current: DeviceInfo by lazy {
DeviceInfo(Build.MANUFACTURER, Build.MODEL)
}
}
}

Expand Down Expand Up @@ -122,7 +123,7 @@ class EInkDeviceIdentifier {
DeviceInfo("barnesandnoble", "bnrv520"),
DeviceInfo("barnesandnoble", "bnrv700"),
DeviceInfo("barnesandnoble", "evk_mx6s1"),
DeviceInfo("barnesandnoble","ereader"), // probably a eink device
DeviceInfo("barnesandnoble", "ereader"), // Probably an eink device
DeviceInfo("freescale", "bnrv510"),
DeviceInfo("freescale", "bnrv520"),
DeviceInfo("freescale", "bnrv700"),
Expand All @@ -134,20 +135,20 @@ class EInkDeviceIdentifier {
DeviceInfo("sony", "dpt-rp1"),
DeviceInfo("onyx", "tagus_pokep"),
DeviceInfo("xiaomi", "xiaomi_reader"),
DeviceInfo("artatech", "pri"), // probably a eink device
DeviceInfo("crema", "crema-0710c"), // probably a eink device
DeviceInfo("crema", "crema-0670c"), // probably a eink device
DeviceInfo("artatech", "pri"), // Probably an eink device
DeviceInfo("crema", "crema-0710c"), // Probably an eink device
DeviceInfo("crema", "crema-0670c"), // Probably an eink device

// Source: https://github.com/plotn/coolreader/blob/e5baf0607e678468aa045053ba5f092164aa1dd7/android/src/org/coolreader/crengine/DeviceInfo.java
DeviceInfo("barnesandnoble", "NOOK"),
DeviceInfo("barnesandnoble", "bnrv350"),
DeviceInfo("barnesandnoble", "bnrv300"),
DeviceInfo("barnesandnoble", "bnrv500"),
DeviceInfo("sony", "PRS-T"), // probably a eink device
DeviceInfo("sony", "PRS-T"), // Probably an eink device
DeviceInfo("dns", "DNS Airbook EGH"),

// Source: https://github.com/ankidroid/Anki-Android/issues/17618
DeviceInfo("Viwoods", "Viwoods AiPaper"),
DeviceInfo("Viwoods", "Viwoods AiPaper")
)

private val eInkManufacturersList = setOf(
Expand All @@ -167,7 +168,7 @@ class EInkDeviceIdentifier {
"dns",
"crema",
"kindle",
"bigme",
"bigme"
)

/**
Expand All @@ -185,15 +186,17 @@ class EInkDeviceIdentifier {
for (device in knownEInkDevices) {
// Check if the device is an exact match.
if (currentDevice.manufacturer == device.manufacturer &&
currentDevice.model == device.model) {
currentDevice.model == device.model
) {
isExactMatch = true
break
}
// Check if the device is a partial match. Partial matches are detected using substring matching.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if ((currentDevice.manufacturer.contains(device.manufacturer) ||
device.manufacturer.contains(currentDevice.manufacturer)) &&
(currentDevice.model.contains(device.model) ||
device.model.contains(currentDevice.model))) {
device.model.contains(currentDevice.model))
) {
isPartialMatch = true
}
}
Expand All @@ -202,7 +205,7 @@ class EInkDeviceIdentifier {
Timber.d("Confirmed E-ink device: $currentDevice")
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 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)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Timber.w("Potential E-ink device: $currentDevice")
ACRA.errorReporter.handleSilentException(Exception("Potential E-ink device: $currentDevice"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use

               CrashReportService.sendExceptionReport(
                    onlyIfSilent = true
                )

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

Copy link
Contributor Author

@Scapesfear Scapesfear Jan 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

like this right ?

CrashReportService.sendExceptionReport(
                Exception("Potential E-ink device: ${Build.MANUFACTURER} | ${Build.BRAND} | ${Build.DEVICE} | ${Build.PRODUCT} | ${Build.MODEL} | ${Build.HARDWARE}"),
                origin = "EInkDeviceIdentifier",
                additionalInfo = null,
                onlyIfSilent = true
            )

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added better diagnostics in: #17686

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

Expand Down
Loading