Skip to content

Conversation

@Thalley
Copy link
Contributor

@Thalley Thalley commented Nov 1, 2024

Since the values are defined as hex, e.g. 0x82, it is easier to compare with the log if they also log them as such.

@zephyrbot zephyrbot added area: Bluetooth area: Bluetooth Qualification Bluetooth Qualification -related issues and pull requests labels Nov 1, 2024
@zephyrbot zephyrbot requested review from jhedberg and sjanc November 1, 2024 14:05
jhedberg
jhedberg previously approved these changes Nov 1, 2024
Copy link
Contributor

Choose a reason for hiding this comment

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

services are defined as decimal

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🙃

@Thalley Thalley added the DNM This PR should not be merged (Do Not Merge) label Nov 3, 2024
@Thalley
Copy link
Contributor Author

Thalley commented Nov 3, 2024

@sjanc
In Zephyr the services are defined as decimal, opcodes and error codes are hex, indexes I'm not sure

In the autopts client everything seems to be logged as decimal, e.g. Header(svc_id=0, op=128, ctrl_index=255, data_len=0), but where services are (luckily) also decimal and opcodes are hex.

We really should align these so that it's possible to compare not only the definitions, but also the logs.

Is there a reason why the services are decimal?

@Thalley Thalley removed the DNM This PR should not be merged (Do Not Merge) label Nov 3, 2024
sjanc
sjanc previously approved these changes Nov 4, 2024
Copy link
Contributor

@sjanc sjanc left a comment

Choose a reason for hiding this comment

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

no reason (other than it is defined like this in autopts docs), we should just switch to hex everywhere

@Thalley Thalley added this to the v4.1.0 milestone Nov 4, 2024
kruithofa
kruithofa previously approved these changes Nov 4, 2024
Since the values are defined as hex, e.g. 0x82, it is easier
to compare with the log if they also log them as such.

Signed-off-by: Emil Gydesen <[email protected]>
Everything else is defined as hex, so it makes sense to
be consistent. This will also make it easier to find the
service IDs in the logs that primarily already log
commands and events as hex.

Signed-off-by: Emil Gydesen <[email protected]>
@Thalley Thalley dismissed stale reviews from kruithofa and sjanc via c62cac5 November 5, 2024 15:56
@Thalley
Copy link
Contributor Author

Thalley commented Nov 5, 2024

Since auto-pts/auto-pts#1292 got merged in the auto-pts repo, I pulled in the commit to also change the service to be hexidecimal in this PR.

@Thalley Thalley requested review from kruithofa and sjanc November 5, 2024 15:57
@nashif nashif merged commit 75f5f71 into zephyrproject-rtos:main Nov 16, 2024
17 checks passed
@Thalley Thalley deleted the btp_logs_update branch November 16, 2024 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Bluetooth Qualification Bluetooth Qualification -related issues and pull requests area: Bluetooth

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants