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

Add Motorola G3, edit some other Motorola values #277

Merged
merged 1 commit into from
Aug 26, 2023
Merged

Conversation

JoesCat
Copy link
Collaborator

@JoesCat JoesCat commented Aug 23, 2023

Added values according to Motorola G3.

adb,charging
Bus 001 Device 014: ID 22b8:2e81 Motorola PCS
charging
Bus 001 Device 040: ID 22b8:2e82 Motorola PCS

adb,mtp
Bus 001 Device 018: ID 22b8:2e76 Motorola PCS
mtp
Bus 001 Device 040: ID 22b8:2e82 Motorola PCS

adb,ptp
Bus 001 Device 031: ID 22b8:2e84 Motorola PCS
ptp
Bus 001 Device 039: ID 22b8:2e83 Motorola PCS

adb,rndis
Bus 001 Device 027: ID 22b8:2e25 Motorola PCS
rndis
Bus 001 Device 038: ID 22b8:2e24 Motorola PCS

adb, audio sources
Bus 001 Device 028: ID 18d1:2d03 Google Inc.
audio sources
Bus 001 Device 035: ID 18d1:2d02 Google Inc.

adb,midi
Bus 001 Device 029: ID 18d1:4ee9 Google Inc.
midi
Bus 001 Device 034: ID 18d1:4ee8 Google Inc.

Fetched some related idProducts from libmtp.

# Moto Z3 Play, beckham, XT1929
# For details see: <https://github.com/M0Rf30/android-udev-rules/issues/264>
# 18d1:4ee8=midi; 18d1:4ee9=midi+adb
# BP TOOLS: 2ee5=charging, mtp, ptp; 2ee6=charging+adb, mtp+adb, ptp+adb; 2ee7=rndis; 2ee8=rndis+adb; 18d1:4ee8=midi; 18d1:4ee9=midi+adb
# QCOM: 05c6:9091=charing+adb, mtp+adb, ptp+adb; 05c6:9092=charging, mtp, ptp; 18d1:4ee8=midi; 18d1:4ee9=midi+adb; 22b8:2e24=rndis; 22b8:2e25=rndis+adb
ATTR{idProduct}=="2e24", GOTO="rndis"
ATTR{idProduct}=="2e25", GOTO="adbrndis"
ATTR{idProduct}=="2e33", GOTO="adbmtp"
ATTR{idProduct}=="2e51", GOTO="adbmtp"
Copy link
Contributor

@mskiptr mskiptr Aug 23, 2023

Choose a reason for hiding this comment

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

I'm not sure that's feasible but could we try to keep individual devices separated? Ideally each device would be in its own section that fully describes what usb ids (and in which modes) it can show up as.

Of course since manufacturers do use the same IDs for different devices, we will have to somehow accommodate that, so here's an idea:

For Moto Z3 Play and Galaxy A4 (2016) tried keeping the following format:

#   <device name>, <codename>, <model #>
#     <any additional information, like links to a full investigation>
#     <ids that already appear elsewhere or that we can't yet reasonably use in code>
#     <weird case>: <ids that appear there>
<rule0>
<rule1> # also <another mode that features the same id, but we don't have a way to express in code or distinguish it>
<rule2> # <explanation if the rule doesn't exactly match the mode the device is in>

The first two lines are pretty self-explanatory.

The next two lines are in this format:

<pid0>=<mode1>+<mode2>, <mode3>; <vid>:<pid1>=<mode4>

where pluses mean multiple modes apply at the same time, commas mean several different modes (or mode combinations) give the same vid:pid pair and semicolons separate unrelated cases (where vid:pid was different).

Finally there are the actual rules.

  • If there's exactly one mode combination that yields this id, there's no need for further comments.
  • If there are several cases that yield this id, the rule should specify the most general one and all the others should be listed in the comment that follows. (like for 22b8:2e8{1,2})
  • If there are several cases that yield this id, but nothing really fits we should do something reasonable and explain what it really means in the comment. (like 22b8:2eb7 or 04e8:6860)

So, what to do when there's already a rule for a given id but as part of a different device? I think we should just leave it there and specify this case in the comment. I tried to do that for the modes that use ids from Google: 18d1:2d0{2,3} and 18d1:4ee{8,9}. If it becomes clear that an ID is consistent across many devices, we can move it out of the first one into some vendor-wide section (and in return put a comment in the device-specific section).

@mskiptr
Copy link
Contributor

mskiptr commented Aug 23, 2023

Also, could we slow down a bit and keep all these related commits in one PR instead of dozens? I think @M0Rf30 is getting a bit overwhelmed by the volume of changes.

Actually, would it be possible to move main at least 5 commits back and instead keep these recent patches in a feature branch for a while? If we could work on them a little bit more, maybe we will be able to find a good strategy for managing where the data comes from and how uncertain it is.

@M0Rf30
Copy link
Owner

M0Rf30 commented Aug 23, 2023

Also, could we slow down a bit and keep all these related commits in one PR instead of dozens? I think @M0Rf30 is getting a bit overwhelmed by the volume of changes.

Actually, would it be possible to move main at least 5 commits back and instead keep these recent patches in a feature branch for a while? If we could work on them a little bit more, maybe we will be able to find a good strategy for managing where the data comes from and how uncertain it is.

No issues from my side. we will consolidate after. I will wait a little bit of time before tag. For now I only ask (especially to @JoesCat ) to use conventional commit messages (it's the third time that I gently ask for this and I'm ignored).
Thanks for your precious efforts.

@mskiptr
Copy link
Contributor

mskiptr commented Aug 23, 2023

Hmm, then will you accept PRs that are rewriting history?

Added values according to Motorola G3.
Fetched some related idProducts from libmtp.
@JoesCat
Copy link
Collaborator Author

JoesCat commented Aug 26, 2023

Ouch - seems I did a force push while you were editing this..... I'll go push some other code.... sorry about that. bad timing by chance :-/

@JoesCat
Copy link
Collaborator Author

JoesCat commented Aug 26, 2023

Hmm, then will you accept PRs that are rewriting history?

@mskiptr - I noticed earlier you are creating patches on your "main" branch.
There is nothing wrong with that if/when you are developing as a lone-developer, or when the churn of new patches is slow enough that you can squeeze-out a patch before the next one comes along. It does become a problem when there are more than one developer involved, and several developers inserting patches all at the same time.

It is probably a good time to learn about how to create git "branches".
This will help you in the long run, since you can then continue fixing your patch together without much worry about someone else squeezing in a patch that messes-up your future patch (like I did with the 5+ patches which weren't related.
https://github.com/mskiptr/android-udev-rules/network
The several patches I gave were all branches that could be merged higher. You can still see the branches here since @M0Rf30 did a squash merge on them, and if you look a little further back in time, you could see @M0Rf30 did a regular merge with your code, so the merge looks a little different.
There are a few git commands you can do on the command line worth knowing.

@M0Rf30 M0Rf30 merged commit 0c84190 into M0Rf30:main Aug 26, 2023
@JoesCat
Copy link
Collaborator Author

JoesCat commented Aug 28, 2023

Hi @M0Rf30 - I think I've flogged all the code I could with the last patch being "misc".
A lot of code behaviour changed from assuming yes (setting env value before testing products), to assuming no (test products and GOTO else exit to "end").

There were a few vendors that I had to leave-alone for now since they contain a lot of question marks (example sonyericsson and a couple others). Maybe best to see reaction to this breakage, and leave the sonyericsson and other fixes for later.

Hi @mskiptr - sorry about the rapid large number of patches sent. I thought it best to get as much of it done at once before the next TAG point. Hopefully you can recover from this point without too much trouble.

JoesCat added a commit to JoesCat/libmtp that referenced this pull request Oct 1, 2023
Add documentation to help, otherwise, many IDs already listed.
M0Rf30/android-udev-rules#264
M0Rf30/android-udev-rules#277
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants