Skip to content

export device properties#383

Merged
pazos merged 1 commit into
koreader:masterfrom
pazos:device_properties
Sep 16, 2022
Merged

export device properties#383
pazos merged 1 commit into
koreader:masterfrom
pazos:device_properties

Conversation

@pazos

@pazos pazos commented Sep 15, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Same intent as #381 but exporting them all.

It makes possible to have very specific branches to workaround broken aosp devices, like

if android.prop.is_tolino then
    if android.prop.hardware ~= "whatever" or android.prop.model ~= "device" then
        -- stuff for all tolino devices except whatever/device
    end
end

exports manufacturer, brand, device, model, product and hardware as strings and boolean values for is_tolino and is_boyue as both brands use some non AOSP keycodes.

This change is Reviewable

@pazos pazos mentioned this pull request Sep 16, 2022
Closed
@pazos pazos requested a review from Frenzie September 16, 2022 08:29
@hasezoey hasezoey mentioned this pull request Sep 16, 2022
Merged
hasezoey
hasezoey reviewed Sep 16, 2022
View reviewed changes

@hasezoey hasezoey left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

while you are at it, i would suggest changing the current TOLINO conditions to instead of just DEVICE.contentEquals("ntx_6sl")) to also check for MODEL.contentEquals("tolino") or only MODEL.contentEquals("tolino") (no other conditions)
also would suggest re-using TOLINO for the check for TOLINO_EPOS

for example the Vision5 getprop for ro.prop is

[ro.product.board]: [EVK]
[ro.product.brand]: [RakutenKobo]
[ro.product.cpu.abi2]: [armeabi]
[ro.product.cpu.abi]: [armeabi-v7a]
[ro.product.device]: [ntx_6sl]
[ro.product.display]: [eink]
[ro.product.hardwareType]: [E70K00]
[ro.product.lightGestureEnabled]: [true]
[ro.product.locale.language]: [en]
[ro.product.locale.region]: [US]
[ro.product.manufacturer]: [Rakuten Kobo Inc.]
[ro.product.model]: [tolino]
[ro.product.name]: [ntx_6sl]

@pazos

pazos commented Sep 16, 2022

Copy link
Copy Markdown
Member Author

Not sure what your intention is here :/

TOLINO is a catch-them all condition. It should work on all fws for all tolino devices, even on third party firmware like Ryogo-Z. It should not catch tolino devices that are not eink.

I won't plan to change that.

I'm also won't plan to change any other device check without a device owner reporting the corresponding stuff. And in that case I would add the new condition as an OR and never replace the check because that might screw things on previous devices that are already working fine.

@hasezoey

hasezoey commented Sep 16, 2022

Copy link
Copy Markdown
Contributor

Not sure what your intention is here :/
I won't plan to change that.

i though i might as well ask, because this would bring TOLINO and TOLINO_EPOS conditions (and coding style) in line with other things (like BOYUE)

as for updating the conditions on TOLINO, i though i also might as well ask to bring it in line with things like the BOYUE conditions and reduce the OR conditions because they will be similar

see

android-luajit-launcher/app/src/main/java/org/koreader/launcher/device/DeviceInfo.kt

Lines 378 to 381 in 58cb837

// Tolino (catch them all)
TOLINO = BRAND.contentEquals("tolino") && MODEL.contentEquals("imx50_rdp")
|| MODEL.contentEquals("tolino") && (DEVICE.contentEquals("tolino_vision2")
|| DEVICE.contentEquals("ntx_6sl"))

android-luajit-launcher/app/src/main/java/org/koreader/launcher/device/DeviceInfo.kt

Lines 383 to 386 in 58cb837

// Tolino Epos 2 and Tolino Vision 4 also have warmth lights
TOLINO_EPOS = BRAND.contentEquals("rakutenkobo")
&& MODEL.contentEquals("tolino")
&& DEVICE.contentEquals("ntx_6sl")

android-luajit-launcher/app/src/main/java/org/koreader/launcher/device/DeviceInfo.kt

Lines 174 to 175 in 58cb837

IS_BOYUE = MANUFACTURER.contentEquals("boeye")
|| MANUFACTURER.contentEquals("boyue")

Frenzie
Frenzie approved these changes Sep 16, 2022
View reviewed changes

@Frenzie Frenzie left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Annoying, but seems sensible enough.

@pazos pazos merged commit a3540de into koreader:master Sep 16, 2022
@pazos pazos deleted the device_properties branch September 16, 2022 10:27
@NiLuJe

NiLuJe commented Sep 16, 2022

Copy link
Copy Markdown
Member

Not breaking backward compatibility with brittle crap like this unfortunately trumps coding style ;).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Frenzie Frenzie Frenzie approved these changes
+1 more reviewer
@hasezoey hasezoey hasezoey left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@pazos @hasezoey @NiLuJe @Frenzie