PixelOS-AOSP / official_devices

55 stars 60 forks source link

New maintainer #347

Closed flakeforever closed 9 months ago

flakeforever commented 9 months ago

Name and Codename of the device you want to apply for

Poco F5 Pro / Redmi K60 (mondrian)

Device Tree sources

https://github.com/flakeforever/device_xiaomi_mondrian
https://github.com/flakeforever/device_xiaomi_mondrian-kernel
https://github.com/flakeforever/vendor_xiaomi_mondrian
https://github.com/flakeforever/kernel_xiaomi_mondrian
https://github.com/PixelOS-AOSP/hardware_xiaomi

What ROM's do you maintain officially/unofficially

PixelOS 14
Pixel Experience Plus 13

How long have you been building Custom ROMs

More than 5 years

Any Exceptions/special concern?

No

List of changes/patches applied to source, if any

https://github.com/flakeforever/device_xiaomi_mondrian/tree/staging/patches

Contact

Telegram username

seadeepdeep

Link to your XDA thread of unofficial build

https://xdaforums.com/t/rom-14-unofficial-pixelos-14-for-poco-f5-pro-redmi-k60.4655397/
flakeforever commented 9 months ago

The contributor seadeepdeep, this is my another name in the virtual machine (also me).

kawaaii commented 9 months ago
  1. You've said you have been engaging in the android development since 5 years, however your Github profiles doesn't seem to reflect much.

  2. The patches you've mentioned in https://github.com/flakeforever/device_xiaomi_mondrian/tree/fourteen/patches, are mostly unrequired. For eg: in display HAL, If you want to use displayfeature, use prebuilt display composer from stock, as there is no proper support for displayfeature in OSS Display HAL. Similarly, do the changes of display init script changes in device tree rather than touching HAL's init scripts.

  3. Furthermore, your commits have missing authorship, and that behaviour is strictly not tolerated.

  4. The shims introduced in https://github.com/flakeforever/device_xiaomi_mondrian/commit/6df42d86f61a811e0e2e342eec36cac466f4db56 are supposed to be used in MiuiCamera repo, rather than in the device tree itself.

  5. https://github.com/flakeforever/device_xiaomi_mondrian/commit/59ad0ac9fc2379b8541db79b7ad873982fadcf44 Please redo your SEPolicy changes, there are quite few changes that I am concerned about, and I think it's really unrequired. For eg: allow bluetooth hal_audio:fd ; allow bluetooth servicemanager:fd ; allow bluetooth system_app:fd ; allow vendor_hal_camerapostproc_xiaomi platform_app:fd ; allow vendor_hal_camerapostproc_xiaomi priv_app:fd ; allow vendor_hal_camerapostproc_xiaomi system_app:fd ;

Looking at the changes and a long internal discussion, we have decided to reject your application for official PixelOS maintainership, as we fail to find the qualifying features that we want to see in our team members/maintainers. I hope you correct these issues that I have mentioned and apply again after getting more experience.

flakeforever commented 9 months ago

I think you can give me advice on where to improve, outright rejection seems very rude. I don't want to use my work account to maintain this project, so I created this account. Regarding missing authorship, I don't understand what you mean. Many of them are organized by myself. You should take a serious look at some of the code I made for this project. For example, with https://github.com/flakeforever/device_xiaomi_mondrian/blob/fourteen/udfps/UdfpsHandler.cpp, I reverse-engineered Xiaomi's mfp-daemon to achieve the same mechanism. It is currently the most complete solution on sm8450, sm8475, and sm8550. I understand that these review will take up your time, but it also allow me to see your abilities.

kawaaii commented 9 months ago

Generally, when you decommonize, that’s done from the common tree. An example is https://github.com/Ad1tyaS1ngh/device_xiaomi_davinci/commit/d56830b8c65ed0d9d2f076aa248ba42ba6b1b880.

flakeforever commented 9 months ago

Alright, none of these are a problem for me. I believe I can do better if I can understand more requirements before applying. I will redo my device project to meet the requirements. Thank you.

kawaaii commented 9 months ago

Did you “reverse-engineer Xiaomi's mfp-daemon to achieve the same mechanism."? Arian did - https://github.com/cupid-development/android_device_xiaomi_sm8450-common/commits/mondrian/udfps/UdfpsHandler.cpp You just added support for brightness_clone, and changed localhbm handling impl which seems like unnecessary change.

“It is currently the most complete solution on sm8450, sm8475, and sm8550.” How is it better? It works perfectly on other devices.

flakeforever commented 9 months ago

Arian made a correction to a constant error definition, but I corrected this error earlier than him (with chat records). In devices like sm8450 and sm8550, there are two types of fingerprint hardware: goodix and fpc. Goodix has excellent fault tolerance, so much so that it seems to be able to run the solution that comes from Lineage. But, for the more demanding fpc devices, it hardly works. After releasing the first ROM, many FPC users reported that the fingerprint was not working, and this issue also appeared in other ROMs. The advantage of my solution is that it fully follows the working mechanism of mfp-daemon, including some asynchronous execution sequences. From the order of the logs, they are consistent. FPC devices can work like they do under MIUI (all of this requires stock kernel modules and display features). For brightness_clone, it seems redundant, but I just followed mfp-daemon completely. Arian's current solution uses an OSS driver. For FPC devices, if users need to enroll fingerprint, they must keep the screen at maximum brightness (I received this information from other users; I have not verified it); otherwise, enrollment will fail.

flakeforever commented 9 months ago

I don't have an fpc device to test this issue, so I had to reverse engineer it, follow it, and then it worked.

flakeforever commented 9 months ago

Anyway, I brought up this example just to show that I have the ability to solve some problems. Maybe my project's format is not so standardized, but I am not lacking in experience.

Since making the ROM of POCO F5 Pro, I have not asked anyone for help in solving problems. I have resolved all issues on my own, so you don't have to worry about me taking up anyone's time.

And my device project should have been ahead of Arian's project (which can be proven from the time I released the first ROM), solely because it contained too many unattributed contents, so I gave it up. As for Arian's project, it appears to be open-source, but it differs significantly from the released version (I interpret this behavior as a lack of timely code updates). I have solved many issues on my own, all based on my original device project. And I am trying to attribute sources.

flakeforever commented 9 months ago

Some people interested in my project may be more concerned about whether it can be built and run. At this point, I believe I am doing well

flakeforever commented 9 months ago

Hi kawaaii, I have refactored my device project based on your advice.

It include the following changes: 1.I have annotated all changes with the authorship whenever possible, If those contents was copied from someone else.

2.Merged all steps of 'decommonize' into one step. https://github.com/flakeforever/device_xiaomi_mondrian/commit/38f4551d30652168836384b6c440832e90d867e5

3.Removed the display HAL patch and moved some files to the 'configs' directory. https://github.com/flakeforever/device_xiaomi_mondrian/tree/staging/configs/display

4.Removed the shims, which will be used in MiuiCamera repo.

5.Removed unused sepolicy rules. https://github.com/flakeforever/device_xiaomi_mondrian/commit/4fe208af0e897cfa710ccdad16a9d8a195ca5873

6.Removed the prebuilt kernel and created the mondrian-kernel repository, following Marbel's reference. https://github.com/flakeforever/device_xiaomi_mondrian-kernel

7.Moved all configuration files into the 'configs' directory (audio display vintf etc.). https://github.com/flakeforever/device_xiaomi_mondrian/commit/b0098d667e05fc3a18c7475f0dac26f454a0ae60

8.Through the process of refactoring, I have made many adjustments to details that were not listed.

I have tested them to build and run, they are working. Please further review my project. If there are any adjustments needed, just let me know. Thank you.

cyberknight777 commented 9 months ago

Are you building the kernel or shipping it as a prebuilt?

flakeforever commented 9 months ago

It's a prebuilt from https://github.com/LowTension/android_kernel_xiaomi_sm8475. And only the Image (kernel) is built from this repo. Kernel modules are all from stock.

cyberknight777 commented 9 months ago

You could just build the kernel inline and ship stock modules no?

flakeforever commented 9 months ago

Okay, I will build it inline.

cyberknight777 commented 9 months ago

Hello there, regarding the source patches you have in your tree;

As for hardware/xiaomi, are you not utilizing our hardware/xiaomi which is at github.com/PixelOS-AOSP/hardware_xiaomi ? If you're not then apply udfps sensor patches in there, otherwise;

What was the purpose of removing uhid at here (https://github.com/flakeforever/device_xiaomi_mondrian/blob/staging/patches/hardware_xiaomi/0002-Change-rc-file-for-sensors-service.diff#L11) ?

Is that variable (https://github.com/flakeforever/device_xiaomi_mondrian/blob/staging/patches/hardware_xiaomi/0001-Extend-the-definition-of-UdfpsHandler.diff#L12) not defined in the display HAL?

As for Settings,

You wouldn't need this (https://github.com/flakeforever/device_xiaomi_mondrian/blob/staging/patches/packages_apps_Settings/0001-Fix-default-min-refresh-rate.diff) if your minimum refresh rate has been detected properly by this (https://github.com/PixelOS-AOSP/packages_apps_Settings/commit/5cce1d962deaccd4ae880a9b221f2e9afe83ed49#diff-c3570c9bd27a7ffc3e980955145334240515cc2dd3bef74620081aeff4d7c6beR92). So figure out why it doesn't detect it instead of changing the default.

cyberknight777 commented 9 months ago

As for your device tree in general;

https://github.com/flakeforever/device_xiaomi_mondrian/commit/b601052f9c930724bbc12b07d39b07bc1adac1d4#diff-418ab0ca9d9051dd0b754e5b28af9bb5d001cf3a36c9a097393dc0d4a1ce5acbR50 I'm pretty sure you don't need to set that, also import with proper authorship and history.

https://github.com/flakeforever/device_xiaomi_mondrian/commit/ff0a37bdef44bc24aaa6a940ef8c062deca37e62#diff-05b699c47a2ae1e03e4cd94f7e8226a7514ca410d79badf937f5efa1200c9fe7R1440-R1444 You don't need to ship these as they're irrelevant to your device panels. Only ship QDCM calibration XMLs that are for your device.

https://github.com/flakeforever/device_xiaomi_mondrian/commit/957ccd56b482dcbbc17cd0ddf8854623787facc5 Instead of doing it like this, commit it separately with proper authorship from that tree.

https://github.com/flakeforever/device_xiaomi_mondrian/commit/40d7e61613da12e7d13c38ca961a17ab145cd97b Same goes to this as you should author the person who patched these files.

https://github.com/flakeforever/device_xiaomi_mondrian/commit/3ca6d3eec2b90c6b1e3ebf396ecbc00d47b60195 Amend this with the proper author instead.

https://github.com/flakeforever/device_xiaomi_mondrian/commit/c61ea7da4cbdf0c5d5d630ed52e9ec15a2a2e95d Actually author the person instead of saying that the person authored it.

https://github.com/flakeforever/device_xiaomi_mondrian/commit/4b93420889604adf2a286d8e9de717ae811fa08e Do provide authorship to the respective authors.

https://github.com/flakeforever/device_xiaomi_mondrian/commit/dc5355a368b4e95b6f6e0862971d8bb23be1b66f Authorship is a concern here aswell as why this is needed since you are already building the lite variant of libprotobuf which makes this redundant.

https://github.com/flakeforever/device_xiaomi_mondrian/commit/62a511057895eb62b2dac303652027b72a50c5fb Squash this with PixelOS bringup commit.

https://github.com/flakeforever/device_xiaomi_mondrian/commit/6f6fd4675c434a821427e1c196960de8fcca5576 You can just do this instead: https://github.com/StatiXOS/android_device_xiaomi_sky/commit/c313cf0a0a5c9c27452e30330977ae44b14f115c

https://github.com/flakeforever/device_xiaomi_mondrian/commit/6d0206a61abba66af0657e846691f6dc1b315358 why remove it at all?

https://github.com/flakeforever/device_xiaomi_mondrian/commit/7fdf29fd0c56464c721493a5c86b9d12ea522a5d Fix authorship

Fix authorship for these aswell: [1] https://github.com/flakeforever/device_xiaomi_mondrian/commit/3a3da562181fa3fc654986f506e8f47a8f220d90 [2] https://github.com/flakeforever/device_xiaomi_mondrian/commit/fd9e12492c065377eaf66d9d45f94252ffd611d4

flakeforever commented 9 months ago

Thank you for your correction.

_As for hardware/xiaomi, are you not utilizing our hardware/xiaomi which is at github.com/PixelOS-AOSP/hardwarexiaomi ? If you're not then apply udfps sensor patches in there, otherwise;

I will add hardware dependencies to github.com/PixelOS-AOSP/hardware_xiaomi.

_What was the purpose of removing uhid at here (https://github.com/flakeforever/device_xiaomi_mondrian/blob/staging/patches/hardware_xiaomi/0002-Change-rc-file-for-sensors-service.diff#L11) ?_

It is indeed unnecessary; it is a legacy issue. I will remove it.

_Is that variable (https://github.com/flakeforever/device_xiaomi_mondrian/blob/staging/patches/hardware_xiaomi/0001-Extend-the-definition-of-UdfpsHandler.diff#L12) not defined in the display HAL?_

This is not involved in compilation; I will remove it.

As for Settings,

_You wouldn't need this (https://github.com/flakeforever/device_xiaomi_mondrian/blob/staging/patches/packages_apps_Settings/0001-Fix-default-min-refresh-rate.diff) if your minimum refresh rate has been detected properly by this (https://github.com/PixelOS-AOSP/packages_apps_Settings/commit/5cce1d962deaccd4ae880a9b221f2e9afe83ed49#diff-c3570c9bd27a7ffc3e980955145334240515cc2dd3bef74620081aeff4d7c6beR92). So figure out why it doesn't detect it instead of changing the default._

I only configured a refresh rate setting without setting both the maximum and minimum, and then I found out that adaptive was enabled by default. I think if I set both the minimum and maximum refresh rates to 120 in the overlay, then this issue should not occur? Okay, I will also remove this.

flakeforever commented 9 months ago

In progress

flakeforever commented 9 months ago

_As for hardware/xiaomi, are you not utilizing our hardware/xiaomi which is at github.com/PixelOS-AOSP/hardwarexiaomi ? If you're not then apply udfps sensor patches in there, otherwise;

Switched to github.com/PixelOS-AOSP/hardware_xiaom

_What was the purpose of removing uhid at here (https://github.com/flakeforever/device_xiaomi_mondrian/blob/staging/patches/hardware_xiaomi/0002-Change-rc-file-for-sensors-service.diff#L11) ?_

Removed it.

_Is that variable (https://github.com/flakeforever/device_xiaomi_mondrian/blob/staging/patches/hardware_xiaomi/0001-Extend-the-definition-of-UdfpsHandler.diff#L12) not defined in the display HAL?_

Removed it.

As for Settings,

_You wouldn't need this (https://github.com/flakeforever/device_xiaomi_mondrian/blob/staging/patches/packages_apps_Settings/0001-Fix-default-min-refresh-rate.diff) if your minimum refresh rate has been detected properly by this (https://github.com/PixelOS-AOSP/packages_apps_Settings/commit/5cce1d962deaccd4ae880a9b221f2e9afe83ed49#diff-c3570c9bd27a7ffc3e980955145334240515cc2dd3bef74620081aeff4d7c6beR92). So figure out why it doesn't detect it instead of changing the default._

Removed it.

new link: https://github.com/flakeforever/device_xiaomi_mondrian/tree/staging/patches

cyberknight777 commented 9 months ago

break down the commits I mentioned such as libinit with proper authorship instead of co-authoring everyone FYI.

flakeforever commented 9 months ago

1._https://github.com/flakeforever/device_xiaomi_mondrian/commit/b601052f9c930724bbc12b07d39b07bc1adac1d4#diff-418ab0ca9d9051dd0b754e5b28af9bb5d001cf3a36c9a097393dc0d4a1ce5acbR50 I'm pretty sure you don't need to set that, also import with proper authorship and history._

Authorship added, SafetyNet workaround removed. new link: https://github.com/flakeforever/device_xiaomi_mondrian/commit/eb956c40bd46ac8209453e8cd8e8b03313f298e9

2._https://github.com/flakeforever/device_xiaomi_mondrian/commit/ff0a37bdef44bc24aaa6a940ef8c062deca37e62#diff-05b699c47a2ae1e03e4cd94f7e8226a7514ca410d79badf937f5efa1200c9fe7R1440-R1444 You don't need to ship these as they're irrelevant to your device panels. Only ship QDCM calibration XMLs that are for your device._

Removed those. new link: https://github.com/flakeforever/device_xiaomi_mondrian/commit/feba6d10b40e0f3cdcc0e3f9ad817795e76edcb0

3._https://github.com/flakeforever/device_xiaomi_mondrian/commit/957ccd56b482dcbbc17cd0ddf8854623787facc5 Instead of doing it like this, commit it separately with proper authorship from that tree._

Added authorship. new link: https://github.com/flakeforever/device_xiaomi_mondrian/commit/2146bfd9f284f2aed03bc0dc5b8d04b70f4df3d1

4._https://github.com/flakeforever/device_xiaomi_mondrian/commit/40d7e61613da12e7d13c38ca961a17ab145cd97b Same goes to this as you should author the person who patched these files._

Added authorship. new link: https://github.com/flakeforever/device_xiaomi_mondrian/commit/d56f9046eea369a2257e4a1f0014751f85fd0137

5._https://github.com/flakeforever/device_xiaomi_mondrian/commit/3ca6d3eec2b90c6b1e3ebf396ecbc00d47b60195 Amend this with the proper author instead._

Removed it, megered to https://github.com/flakeforever/device_xiaomi_mondrian/commit/27b95e2217adbce2151e33d835fded9fdd45424e

6._https://github.com/flakeforever/device_xiaomi_mondrian/commit/c61ea7da4cbdf0c5d5d630ed52e9ec15a2a2e95d Actually author the person instead of saying that the person authored it._

Added authorship. new link: https://github.com/flakeforever/device_xiaomi_mondrian/commit/ef0fe94fbc6c2f7a0cb385c3afa2c286536d97d8

7._https://github.com/flakeforever/device_xiaomi_mondrian/commit/4b93420889604adf2a286d8e9de717ae811fa08e Do provide authorship to the respective authors._

Fixed it. new link: https://github.com/flakeforever/device_xiaomi_mondrian/commit/a08891a2169fc0463b43db22ea81c7122024dc41

8._https://github.com/flakeforever/device_xiaomi_mondrian/commit/dc5355a368b4e95b6f6e0862971d8bb23be1b66f Authorship is a concern here aswell as why this is needed since you are already building the lite variant of libprotobuf which makes this redundant._

I later learned about libprotobuf from you in other thread, so I added libprotobuf. But I forgot to revert these. Fixed it. new link: https://github.com/flakeforever/device_xiaomi_mondrian/commit/0e8742a259004d79b5cf957a609e11071898c152

9._https://github.com/flakeforever/device_xiaomi_mondrian/commit/62a511057895eb62b2dac303652027b72a50c5fb Squash this with PixelOS bringup commit._

Fixed it. new link: https://github.com/flakeforever/device_xiaomi_mondrian/commit/5e23f23d6a0216a2c65926bd0647d126b5f56cd7

10._https://github.com/flakeforever/device_xiaomi_mondrian/commit/6f6fd4675c434a821427e1c196960de8fcca5576 You can just do this instead: https://github.com/StatiXOS/android_device_xiaomi_sky/commit/c313cf0a0a5c9c27452e30330977ae44b14f115c_

I have tried this method, but it still doesn't work. My understanding is that there may be some binary patch here?

11._https://github.com/flakeforever/device_xiaomi_mondrian/commit/6d0206a61abba66af0657e846691f6dc1b315358 why remove it at all?_

I changed it to android.hardware.drm@1.4-service.clearkey.vendor https://github.com/flakeforever/device_xiaomi_mondrian/commit/881abf4a85f41b71cacc612fb9855047b60a6bee

12._https://github.com/flakeforever/device_xiaomi_mondrian/commit/7fdf29fd0c56464c721493a5c86b9d12ea522a5d Fix authorship_

Fixed it. new link: https://github.com/flakeforever/device_xiaomi_mondrian/commit/df0a60e1b57ad8b4bab3f14689953db86c4ecb5a

13._Fix authorship for these aswell: [1] https://github.com/flakeforever/device_xiaomi_mondrian/commit/3a3da562181fa3fc654986f506e8f47a8f220d90_

These dependencies based on my analysis of readelf and ramoops logs.

14.[2] _https://github.com/flakeforever/device_xiaomi_mondrian/commit/fd9e12492c065377eaf66d9d45f94252ffd611d4_

Fixed it. new link: https://github.com/flakeforever/device_xiaomi_mondrian/commit/bcf5c3182c10d8c8afd47566ef13d2a7298e499b

flakeforever commented 9 months ago

https://github.com/PixelOS-AOSP/hardware_xiaomi In this repo, I can only get an 1.0 version of the udfps sensor, and it fails to load when I use the 2.1 HAL service.

Should I possibly port a 2.1 version of the sensor from AOSPA? Or depend on AOSPA/hardware_xiaomi?

cyberknight777 commented 9 months ago

Import 2.x from AOSPA

cyberknight777 commented 9 months ago

@flakeforever you’re not interested anymore ?

flakeforever commented 9 months ago

@cyberknight777 Thank you very much for the time you have put into this, at least I have learned a lot from it. I will continue to maintain my device project to a strict standard. In order to avoid wasting your time, I give up being a maintainer. I'm sorry.