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 pico_get_unique_board_id_string API #281

Merged
merged 3 commits into from
Mar 30, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/rp2_common/pico_stdio_usb/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ if (TARGET tinyusb_device_unmarked)
tinyusb_device_unmarked
pico_stdio
pico_time
pico_unique_id
)

target_compile_definitions(pico_stdio_usb INTERFACE
PICO_STDIO_USB=1
)
endif()
endif()
11 changes: 9 additions & 2 deletions src/rp2_common/pico_stdio_usb/stdio_usb_descriptors.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

#include "tusb.h"
#include "pico/stdio_usb/reset_interface.h"
#include "pico/unique_id.h"

#define USBD_VID (0x2E8A) // Raspberry Pi
#define USBD_PID (0x000a) // Raspberry Pi Pico SDK CDC
Expand Down Expand Up @@ -98,10 +99,12 @@ static const uint8_t usbd_desc_cfg[USBD_DESC_LEN] = {
#endif
};

static char usbd_serial_str[PICO_UNIQUE_BOARD_ID_SIZE_BYTES * 2 + 1];

static const char *const usbd_desc_str[] = {
[USBD_STR_MANUF] = "Raspberry Pi",
[USBD_STR_PRODUCT] = "Pico",
[USBD_STR_SERIAL] = "000000000000", // TODO
[USBD_STR_SERIAL] = usbd_serial_str,
[USBD_STR_CDC] = "Board CDC",
#if PICO_STDIO_USB_ENABLE_RESET_VIA_VENDOR_INTERFACE
[USBD_STR_RPI_RESET] = "Reset",
Expand All @@ -121,6 +124,10 @@ const uint16_t *tud_descriptor_string_cb(uint8_t index, uint16_t langid) {
#define DESC_STR_MAX (20)
static uint16_t desc_str[DESC_STR_MAX];

// Assign the SN using the unique flash id
if (!usbd_serial_str[0])
earlephilhower marked this conversation as resolved.
Show resolved Hide resolved
pico_get_unique_board_id_string(usbd_serial_str, sizeof(usbd_serial_str));

uint8_t len;
if (index == 0) {
desc_str[1] = 0x0409; // supported language is English
Expand All @@ -141,4 +148,4 @@ const uint16_t *tud_descriptor_string_cb(uint8_t index, uint16_t langid) {
return desc_str;
}

#endif
#endif
15 changes: 15 additions & 0 deletions src/rp2_common/pico_unique_id/include/pico/unique_id.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,21 @@ typedef struct {
*/
void pico_get_unique_board_id(pico_unique_board_id_t *id_out);

/*! \brief Get unique ID in string format
* \ingroup pico_unique_id
*
* Get the unique 64-bit device identifier which was retrieved from the
* external NOR flash device at boot, formatted as an ASCII hex string.
* Will always 0-terminate.
*
* On PICO_NO_FLASH builds the unique identifier is set to all 0xEE.
*
* \param id_out a pointer to a char buffer of size len, to which the identifier will be written
* \param len the size of id_out. For full serial, len >= 2 * PICO_UNIQUE_BOARD_ID_SIZE_BYTES + 1
*/
void pico_get_unique_board_id_string(char *id_out, size_t len);


#ifdef __cplusplus
}
#endif
Expand Down
13 changes: 13 additions & 0 deletions src/rp2_common/pico_unique_id/unique_id.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,16 @@ static void __attribute__((constructor)) _retrieve_unique_id_on_boot(void) {
void pico_get_unique_board_id(pico_unique_board_id_t *id_out) {
*id_out = retrieved_id;
}

void pico_get_unique_board_id_string(char *id_out, size_t len) {
Copy link
Contributor

Choose a reason for hiding this comment

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

also for better or worse we've tended to use uint rather than size_t for sizes in APIs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All done. To preserve hex ordering, we actually need to shift (4 - 4 * (i&1)) or the hex nibbles end up backwards.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's a good thing that * has a higher precedence than - 😉

pico_unique_board_id_t temp = retrieved_id;
size_t i;
// Generate hex one nibble at a time
for (i = 0; (i < len - 1) && (i < PICO_UNIQUE_BOARD_ID_SIZE_BYTES * 2); i++) {
size_t bi = i / 2;
int nibble = (temp.id[bi] >> 4) & 0x0F;
temp.id[bi] = (uint8_t)(temp.id[bi] << 4);
Copy link
Contributor

Choose a reason for hiding this comment

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

Apologies if this is a stupid question (I'm not a low-level C programmer), but is this modifying the bytes in the underlying retrieved_id ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this works on the bytes intemp which are copied in line 31.

Copy link
Contributor

Choose a reason for hiding this comment

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

this does feel a bit weird though, i'd be tempted to just do

nibble = (temp.id[i/2] >> (4 * (i&1))) & 0xf;

Copy link
Contributor

Choose a reason for hiding this comment

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

(I guess @kilograham 's suggestion doesn't modify temp, in which case perhaps there's no need for the copy and you can just access retrieved_id?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a much better way and it doesn't need the copy, thanks.

id_out[i] = (char)(nibble < 10 ? nibble + '0' : nibble + 'A' - 10);
}
id_out[i] = 0;
}