sonyxperiadev / device-sony-common

70 stars 139 forks source link

Revert "init: qcrild.rc : Add vendor prefix to ril-daemon" #883

Closed sledges closed 2 years ago

sledges commented 2 years ago

On R the service is still called "ril-daemon".

This fixes random SIM detection https://github.com/sonyxperiadev/bug_tracker/issues/736.

This reverts commit 30b54ba54619cc17bfc8bfd7a56d8e235579431a.

MarijnS95 commented 2 years ago

@sledges We deduced in https://github.com/sonyxperiadev/device-sony-common/pull/874#issuecomment-979126097 that this patch was also applicable to R - can you explain how this is the case? Was that AOSP patch perhaps not picked to the R branch/tag/tree after all?

MarijnS95 commented 2 years ago

Oh and @jerpelea be careful with the rebase script, as this revert will automagically land on s-mr1/master when rebased on top of r-mr1 :grimacing:

sledges commented 2 years ago

@sledges We deduced in #874 (comment) that this patch was also applicable to R - can you explain how this is the case? Was that AOSP patch perhaps not picked to the R branch/tag/tree after all?

There is no /vendor/etc/init/rild.rc on Sony AOSP filesystem, only rild.legacy.rc, within which the service is called ril-daemon.

qcrild.rc attempts to disable vendor.ril-daemon, which fails, and ps -A | grep ril returns 3 running daemons, one of them is the legacy one. Disabling the legacy one properly gets rid of the SIM detection issue.

ix5 commented 2 years ago

Somehow, a ping about this found its way to my inbox.

I can't be bothered about any of the useless waste of everyone's time that is Google dropping new Android versions, but for my humour and the benefit of the sfos people, here goes:

We had planned to align our "properties" to Treble-compliant naming so that we could set PRODUCT_COMPATIBLE_PROPERTY (or rather, PRODUCT_COMPATIBLE_PROPERTY_OVERRIDE) to true. Because of a certain company that starts with Q unwilling to fix their insane mess of proprietary RIL-related code, we could not do this and resorted to exemptions granted to devices with old shipping API levels (hence, why SODP spoofs shipping API levels - I can guarantee that the Xperia 10 Mk. III did not ship with Oreo).

Via some further insanity, the state of "compatible" property names finds its way into the service name of ril-daemon:

On Android 11, which is now nearing 2 years since pre-release and which SODP and sfos somehow still seem to be stuck on, the exemption for "legacy" props based on shipping API or override is still present: https://cs.android.com/android/platform/superproject/+/android-11.0.0_r5:build/make/core/config.mk;l=673-680

However, on A12 and current master releases, it's gone: https://cs.android.com/android/platform/superproject/+/android-12.0.0_r28:build/make/core/config.mk;l=609-612

So this "solution" will not fly with future (actually current) AOSP releases.

/over and out.

jerpelea commented 2 years ago

@MarijnS95 this is why there will be a revert of the revert on top s-mr1 and master

MarijnS95 commented 2 years ago

@MarijnS95 this is why there will be a revert of the revert on top s-mr1 and master

I'd reply with a barbecue emoji if there was one. But since there is none, have a cook instead 🧑‍🍳

There is no /vendor/etc/init/rild.rc on Sony AOSP filesystem, only rild.legacy.rc, within which the service is called ril-daemon.

@sledges that makes it sound like https://github.com/sonyxperiadev/device-sony-common/pull/874 was incomplete and incorrect at best, or did we fix this for S only?

@ix5 Thanks for your insights as always!

(re. shipping API level spoofing, I don't think the "shipping API" of stock has ~much~ any meaning for SODP at all since it "ships" completely different software and really only maintains firmware and a few random blobs in the process)

sledges commented 2 years ago

@sledges that makes it sound like #874 was incomplete and incorrect at best, or did we fix this for S only?

It shouldn't have been applied to R, but back then both branches shared the same git tree, and without the knowledge about rild.rc vs rild.legacy.rc that @ix5 has kindly explained, the split had been averted (until now:).

:hotdog: :hotsprings:

jerpelea commented 2 years ago

@MarijnS95 @sledges the patch has been applied to s-mr1 and master then reverted to prevent future issues

MarijnS95 commented 2 years ago

It shouldn't have been applied to R, but back then both branches shared the same git tree, and without the knowledge about rild.rc vs rild.legacy.rc that @ix5 has kindly explained, the split had been averted (until now:).

In other words we simply misjudged the patch as I haven't even clicked open any of the diffs to see PRODUCT_COMPATIBLE_PROPERTY nor the fallback rild.legacy.rc being added, so the suggestion to backport it to R was incorrect even if the upstream patch landed in the R releases.

@MarijnS95 @sledges the patch has been applied to s-mr1 and master then reverted to prevent future issues

Don't think we should've made it land on R in the first place, but rebasing it off at this stage may only be more confusing history-wise.