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

Use pico_unique_id as the USB device serial number #280

Closed
wants to merge 15 commits into from

Conversation

earlephilhower
Copy link
Contributor

Use the pico_unique_id to generate a unique serial number for the
STDIO_USB device, allowing for individual boards to be identified.

kilograham and others added 13 commits March 8, 2021 12:35
…es missed thus obviating the need for an IRQ but there is an IRQ already pending for another timer (raspberrypi#243)
* pico_stdio_usb: be more explicit about includes, fix warning (raspberrypi#257)

* pico_base: NDEBUG backwards for absolute_time_t (raspberrypi#255)

* pico_util: missing extern C in queue.h (raspberrypi#249)

* build: remove -march which was masking -mcpu, now SVC available (raspberrypi#253)
- Add double quotes because build option it's a string.
- Remove comment as requested by @kilograham.
- Resolves raspberrypi#258.
In order for the returned value to accurately reflect a single moment in time,
ensure the registers are read just once and in the datasheet order.
Before this change, the RTC registers would each be read multiple times,
leading (infrequently) to the returned fields not all reflecting the same
moment in time.

The rp2040 datasheet has what I believe is an incorrect example (embedding
the source of this function); will the datasheet be updated if this function is
fixed?

This problem is only a speculative one; I did not actually observe it in the
wild.
- refactor to reduce duplication
 - add extra param-validation
Use the pico_unique_id to generate a unique serial number for the
STDIO_USB device, allowing for individual boards to be identified.
@lurch
Copy link
Contributor

lurch commented Mar 24, 2021

I thought this was a duplicate, but I did a search and found raspberrypi/debugprobe#8 which is actually slightly different 😆

Can you rebase this against develop-1.1.1 please?

Use the pico_unique_id to generate a unique serial number for the
STDIO_USB device, allowing for individual boards to be identified.
@earlephilhower earlephilhower changed the base branch from master to develop-1.1.1 March 24, 2021 17:44
@earlephilhower
Copy link
Contributor Author

earlephilhower commented Mar 24, 2021

@lurch By inspection it looks like we get the same results. strcat is heavier weight, but this generation is called exactly 1 time and if you've got stdio running then strcat and *printf are along for the ride anyway (i.e. no add'l code space increase).

@lurch
Copy link
Contributor

lurch commented Mar 24, 2021

I wonder if it's worth adding a "common" pico_unique_id_to_string(char* str, uint strlen) function to the pico_unique_id lib? 🤷 (For bonus points maybe you could copy only a subset of the unique_id if strlen is too short, and start repeating the unique_id if strlen is too long? 😆 )

@earlephilhower
Copy link
Contributor Author

As a lowly end user I was hesitant to suggest a new API call, but your suggestion does make sense (modulo the repeating copies if the buffer is too long...that seems like not the right thing to do). Let me adjust this PR accordingly. Then, if merged, picoprobe can be modified to use this new API and only 1 copy of the code will exist (yay!).

pico_unique_board_id_t id;
pico_get_unique_board_id(&id);

for (int i = 0; i < PICO_UNIQUE_BOARD_ID_SIZE_BYTES; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, we should pull in printf etc. just to have the serial number. please replace with a trivial loop to fill in the hex values directly

@kilograham
Copy link
Contributor

ah i see this is concurrent editing... i'm ok with adding a helper function to do the trivial loop since it seems useful.

@kilograham
Copy link
Contributor

also please rebase this onto develop not develop-1.1.1 @lurch steered you wrongly!

@earlephilhower earlephilhower changed the base branch from develop-1.1.1 to develop March 25, 2021 01:45
earlephilhower added a commit to earlephilhower/pico-sdk that referenced this pull request Mar 25, 2021
Add a new API to pico_unique which will turn the unique ID into a
canonical text string.

Use this API to update the USB device serial number in stdio_usb.

Supercedes raspberrypi#280
@earlephilhower
Copy link
Contributor Author

Closing due to being stuck in a weird git rebase heck.

New PR #281 supersedes with the suggested new API and low-code implementation.

kilograham pushed a commit that referenced this pull request Mar 30, 2021
* Add pico_get_unique_board_id_string API

Add a new API to pico_unique which will turn the unique ID into a
canonical text string.

Use this API to update the USB device serial number in stdio_usb.

Supercedes #280

* Clean up -Wconversion=error issues

* Address review comments, fix api typing

Use cleaner binary-to-hex conversion.  Update the length parameter to
use uint per the SDK standard .
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.

7 participants