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

Typing helper #342

Merged
merged 1 commit into from
Nov 29, 2023
Merged

Typing helper #342

merged 1 commit into from
Nov 29, 2023

Conversation

zxzxwu
Copy link
Collaborator

@zxzxwu zxzxwu commented Nov 27, 2023

No description provided.

self.analyzer.emit(
f'{color("L2CAP", "green")} [CID={l2cap_pdu.cid}, '
f'PSM={psm_string}]: {l2cap_pdu.payload.hex()}'
f'PSM={PSM(psm).name}]: {l2cap_pdu.payload.hex()}'
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will fail is the PSM isn't in the map.
The solution here (as well as other places where enums can take on values that aren't fully predictable) is to use this class 43b4d66#diff-6a386c4bf931822c523c3b1559afb51d9a35ce754a5519b770ce3e8606b5d9b4R459
Since that class is part of a PR that's not merged yet, maye we can move its definition to this PR instead so that we can start using it (I would deal with the merge conflict in the other PR)

@zxzxwu
Copy link
Collaborator Author

zxzxwu commented Nov 28, 2023

This PR is not hurry. Reset to draft unless someone needs it.

@zxzxwu zxzxwu marked this pull request as draft November 28, 2023 12:49
self.analyzer.emit(
f'{color("L2CAP", "green")} [CID={l2cap_pdu.cid}, '
f'PSM={psm_string}]: {l2cap_pdu.payload.hex()}'
f'PSM={PSM_NAMES.get(psm)}]: {l2cap_pdu.payload.hex()}'
Copy link
Collaborator

Choose a reason for hiding this comment

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

This won't raise, but it still doesn't do what you need when encountering a PSM that's not in the name map. In the original code, the function name_or_number returns either a name, if found in the map, or a string with the hex number representation of the value. We need to keep support for PSM values not in the map here.

@zxzxwu zxzxwu marked this pull request as ready for review November 29, 2023 16:21
@zxzxwu zxzxwu merged commit 24524d8 into google:main Nov 29, 2023
51 checks passed
@zxzxwu zxzxwu deleted the typing branch December 2, 2023 15:38
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.

2 participants