PixelOS-AOSP / official_devices

54 stars 59 forks source link

New maintainer #299

Closed log1cs closed 11 months ago

log1cs commented 11 months ago

Name and Codename of the device you want to apply for

Nokia 8 (2017, Repartitioned) - codename "NLA"

Device Tree sources

https://github.com/log1cs/android_device_nokia_NLA at branch fourteen
https://github.com/log1cs/kernel_nokia_msm8998 at branch lineage-21
https://github.com/log1cs/android_vendor_nokia_NLA at branch lineage-21

What ROM's do you maintain officially/unofficially

LineageOS (unofficial) - dropped recently
Radiant (not released yet)

How long have you been building Custom ROMs

2021 - so it's ~2 years

Any Exceptions/special concern?

No

List of changes/patches applied to source, if any

(1): https://github.com/log1cs/android_bootable_recovery/commit/3e00ce8d5405d95fe547bfc8bf8736703a6070bd
(2): https://github.com/LycorisOSS/android_vendor_lineage/commit/a3daf33d3702eb29bef5106b3d8bc798f8784628
(3): https://github.com/LineageOS/android_bionic/commit/6bbc9e8a89cd6b85381bedddb256c127123d4772

Two of these are for fixing recovery display. Without these, the device will forever stuck at splash screen whenever user tries to enter recovery. Happens after Android 10 with android::base::unique_fd calls. With A9 legacy fd calls, it works just fine.
Actually, reverting this:
https://github.com/PixelOS-Fourteen/bootable_recovery/commit/f65d48bb5ac611a9dbfba150cfdb0bfc5fd6aaf8 is enough, but other devices still need it, so I guarded it in (1).

(3) is needed to build the OSS camera stack.

Contact

Telegram username

@log1cs

Link to your XDA thread of unofficial build

https://xdaforums.com/t/unofficial-14-nla-retrofit-pixelos-aosp.4646159/

Link to your unofficial PixelOS Build Post other than XDA

https://t.me/NB1_Releases/58
log1cs commented 11 months ago

About the bootable/recovery patch, we've been investigating it since Q. For some reason it also breaks on the A1N (Nokia 8 Sirocco) - same SOC as the Nokia 8 on Q.

Removing O_CLOEXEC (because that's the only noticeable difference I can see) but it doesn't work at all, still stuck at splash. So I've decided to use non auto-closing fd (like (1) and (2)) because that's the only workaround that can make recovery working on this device.

I also tried a few kernel fixes in mdss (even nuking Nokia changes in drivers/video) but all attempt seems to be worthless because none of them was working.

kawaaii commented 11 months ago

1) https://github.com/log1cs/android_device_nokia_NLA/commit/5732b1048269b3f0212619aff9183bbe4e08e3ca

I don't find anything that needs to be corrected here, dropping fqnames is really unrequired. For more infos, you can checkout https://source.android.com/docs/core/architecture/vintf/objects.

2) https://github.com/log1cs/android_device_nokia_NLA/commit/b1acf9cc87139ad7263607056fe4a70a410b05e9 Authoring original author is recommended.

3) https://github.com/log1cs/android_device_nokia_NLA/commit/fb5c1c13b396e02739f03b40c996475afa575865 This change is broken. You should follow this change https://github.com/LineageOS/android_device_fxtec_pro1x/commit/6c9fbc6c275b949b855c02b226b509e5ee7ee9d6. Here, you have imported bluetooth audio configuration without knowing that you already have the entries present in bluetooth audio configuration inside your audio policy configuration.

4) https://github.com/log1cs/android_device_nokia_NLA/commit/01132c98a61ccd471d448e0f0cfc3f3f469ff11b "a2dp_vendor_ldac_decoder: packages/modules/Bluetooth/system/stack/a2dp/a2dp_vendor_ldac_decoder.cc:135 A2DP_VendorLoadDecoderLdac: A2DP_VendorLoadDecoderLdac: cannot open LDAC decoder library libldacBT_bco.so: dlopen failed: library "libldacBT_bco.so" not found". This spam can be ignored and is harmless and you don't have to do dummy for this.

5) https://github.com/log1cs/android_device_nokia_NLA/commit/22e6b9b880ee6f720bf4c269ea559722150f8c8e#diff-57926e2194dd13da1288f0b73f02df19f5bcb30eaac948412eb30c71af538ad9

This is totally wrong, and untrusted apps should not be given access to rootfs directory. Unsure why you would do that in the first place.

Also we would recommend you to add Change-Ids to all the commits you have done thus far, with commit description in each change on why that specific change is done and this also includes authoring original authors.

log1cs commented 11 months ago

Thanks for reviewing. I'm going to fix these up in a few hours.

log1cs commented 11 months ago

About (2), I don't get your point.

The original commit was from here, and it's the same guy who committed that. The reason why it's only partly modified instead of being a full copy-paste is because we're using the A1N_sprout init.qcom.rc, and it already has that line before: https://github.com/log1cs/android_device_nokia_NLA/commit/05a631f68a9cdac720aeea7300b262f298dc4a67#diff-fccec6302c27471369e967c30307e1374a6d019598b4bcc92095b839dce479c5R1121

Here is the original commit: https://github.com/LineageOS/android_device_nubia_msm8998-common/commit/f31214f6dad77aa946de4842ee05e9093da0225a

That's my question. Do you mind clarify it? Thanks

kawaaii commented 11 months ago

@log1cs Checkout https://github.com/LineageOS/android_hardware_interfaces/commits/lineage-20.0/audio/common/all-versions/default/service/android.hardware.audio.service.rc

log1cs commented 11 months ago

Done.

(1) and (4) and (5) are dropped.

About (2), this now splits to two commits: https://github.com/log1cs/android_device_nokia_NLA/commit/ae3569678fee543ffb0b53e6138ae92100de4a86 https://github.com/log1cs/android_device_nokia_NLA/commit/3ad2fa3d1401f0e04fc507c55c291dd2942c7b46

About (3), I did as the suggestions. Extra: There are 2 xmls aren't being used, so i dropped them in https://github.com/log1cs/android_device_nokia_NLA/commit/3eef99b9da536b6b977b44681e8be19ee2ad63d4.

Also I think i gave the authorship to most of the commits that I've missed. Do you mind checking the tree again?

kawaaii commented 11 months ago

Unsure why you would build both QTI perf and libperfmgr at same time. https://github.com/log1cs/android_device_nokia_NLA/blob/fourteen/device.mk#L411-L412

kawaaii commented 11 months ago

Regarding the commits you did, you can just remove the whole QTI BT, along with the flag you kept in BoardConfig Makefile; TARGET_USE_QTI_BT_STACK, and squash commit with commit https://github.com/log1cs/android_device_nokia_NLA/commit/78f2bf545380865f5cd8dc4de7b0ae674d7e5b17, considering you use already use AOSP BT.

Also, rather than just overriding the display ids as 0, using proper display id would be highly recommended. You can find the display id that your device use by doing adb shell dumpsys SurfaceFlinger --display-id.

kawaaii commented 11 months ago

Also, could you provide a bit explanation in the commit https://github.com/log1cs/android_device_nokia_NLA/commit/aac3f79386211dc89c63b6ffb86db4795cf82a0e#diff-f34e049718ed23e48730d31b472167046107834961bbe554a7e57735397adc4dR1-R13 regarding why you have done these changes?

log1cs commented 11 months ago

Also, could you provide a bit explanation in the commit log1cs/android_device_nokia_NLA@aac3f79#diff-f34e049718ed23e48730d31b472167046107834961bbe554a7e57735397adc4dR1-R13 regarding why you have done these changes?

Back when I was debugging about the camera not saving/viewfinder lagging issues, i found out there are tons of untrusted_app denials. These denials are from some Google Camera ports, that was my blind attempt to fix it until I found the fix was way up in bionic.

I thought they fixed something, but they didn't. Until you remind me of that untrusted_app stuff, I was completely ignoring it.

The bionic fixes are from AOSP Master though, and it's merged in A14 source code.

Gonna be removed in the next force push.

log1cs commented 11 months ago

Beside all of the stuff mentioned above, anything else do I have to notice?

kawaaii commented 11 months ago

The overlay wifi_tether_configure_ssid_default should have been inside WifiOverlay. We don't have the support for overlay config_cameraHFRPrivAppList.

log1cs commented 11 months ago

Got it, gonna force push again with these changes.

kawaaii commented 11 months ago

https://github.com/log1cs/android_device_nokia_NLA/blob/fourteen/rootdir/etc/init.qcom.rc#L43-L46 Unrequired.

Considering IORAPd being deprecated, I would suggest dropping these properties as well. https://github.com/log1cs/android_device_nokia_NLA/blob/fourteen/vendor.prop#L273

log1cs commented 11 months ago

I've done all of the thing mentioned above. Please check again.

About the Display-Ids, I have no choice other than override it to 0 just to avoid logspam.

The device launched with Android 7 and ended up at 9, so it does not support have DisplayInfo#unique_id as a stable identifier (https://source.android.com/docs/core/display/multi_display/displays#static).

kawaaii commented 11 months ago

https://github.com/log1cs/android_device_nokia_NLA/blob/fourteen/sepolicy/vendor/untrusted_app.te#L1-L4

Not sure why you would grant untrusted_app these permissions,there should have been better ways to address them.

Apart from this, Consider reapplying later. when you've gained more experiece.