feat: populate MyNodeInfo.device_id on all platforms (#10995)
* feat: populate MyNodeInfo.device_id on all platforms RP2040/RP2350 use the 64-bit pico unique board id, STM32WL the 96-bit silicon UID, ESP32-S2 joins the existing OPTIONAL_UNIQUE_ID efuse branch, and everything else (classic ESP32 in particular) falls back to a deterministic factory-MAC-derived id, resolving the long-standing FIXME. Portduino keeps the config-supplied id preferred and now uses the MAC fallback when the config omits one. No proto or persistence changes; the id is re-read from silicon each boot and PhoneAPI still zeroes it for unauthenticated clients. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: declare zero_mac const to satisfy cppcheck Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: address review feedback on device_id derivation Clear any disk-loaded device_id before the silicon derivation so a failed read leaves it unset rather than stale (Copilot), and size the portduino config copy with sizeof instead of a literal 16 (CodeRabbit). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor: extract device_id generation into per-arch getDeviceId() Per review feedback on #10995: move the platform-specific MyNodeInfo.device_id derivation out of the #if/#elif ladder in NodeDB.cpp into a getDeviceId() interface (target_specific.h) implemented per-architecture alongside each platform's getMacAddr(): - esp32: efuse OPTIONAL_UNIQUE_ID (C3/S2/S3/C6); classic ESP32 -> MAC - nrf52: FICR DEVICEID + DEVICEADDR - nrf54l15: FICR->INFO.DEVICEID + DEVICEADDR (NRF_FICR-guarded, MAC fallback) - rp2xx0: pico_get_unique_board_id() - stm32wl: HAL_GetUIDw0/1/2() - portduino: config-supplied id preferred, else MAC The shared MAC-derived fallback moves to meshUtils as getMacAddrDeviceId(). NodeDB.cpp now zero-inits the field and makes a single getDeviceId() call, dropping ~65 lines of platform boilerplate plus the esp_efuse/pico/stm32 includes that came with it. No behavior change: device_id is still re-read from silicon each boot and never persisted. Builds green: native-macos, tbeam, heltec-v3, rak4631, rak11310, rak3172. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix: address review of device_id refactor - getMacAddrDeviceId(): zero-init the mac[6] buffer. getMacAddr() can return without writing (e.g. Portduino with no MAC source), so the old uninitialized buffer let stack garbage pass the all-zeros guard and become device_id. The pre-refactor code relied on the zero-initialized static ourMacAddr; restore that guarantee. - nrf54l15 getDeviceId(): drop the `#if defined(NRF_FICR)` guard and read FICR unconditionally (as the pre-refactor NodeDB code did). The guarded #else fell back to getMacAddr()'s hard-coded placeholder MAC, which would give every unit an identical device_id; a missing NRF_FICR should be a loud compile error. - NodeDB.cpp: `#include "target_specific.h"` instead of hand-copied externs for getMacAddr/getDeviceId; retire the stale FIXME. Same for meshUtils.cpp (whose extern comment wrongly claimed the TU was Arduino-free). - Delete the orphaned commented-out device_id hex-dump block in NodeDB.cpp. - Trim/de-duplicate the getDeviceId contract comments (single-sourced in target_specific.h). Builds green: native-macos, tbeam. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: trim device_id comments to the repo's 1-2 line guideline Addresses CodeRabbit review nitpick on #10995: shorten the getDeviceId() (target_specific.h), getMacAddrDeviceId() (meshUtils.h), device-id refresh (NodeDB.cpp), and nrf54l15 getDeviceId comments to two lines each, per the "one or two lines maximum" coding guideline. Comment-only; no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
GitHub
Claude Fable 5
parent
fe288a70e4
commit
269b974e96
+8
-52
@@ -27,6 +27,7 @@
|
|||||||
#include "mesh/generated/meshtastic/deviceonly_legacy.pb.h"
|
#include "mesh/generated/meshtastic/deviceonly_legacy.pb.h"
|
||||||
#include "meshUtils.h"
|
#include "meshUtils.h"
|
||||||
#include "modules/NeighborInfoModule.h"
|
#include "modules/NeighborInfoModule.h"
|
||||||
|
#include "target_specific.h"
|
||||||
#if HAS_VARIABLE_HOPS
|
#if HAS_VARIABLE_HOPS
|
||||||
#include "modules/HopScalingModule.h"
|
#include "modules/HopScalingModule.h"
|
||||||
#endif
|
#endif
|
||||||
@@ -50,11 +51,7 @@
|
|||||||
#include "SPILock.h"
|
#include "SPILock.h"
|
||||||
#include "modules/StoreForwardModule.h"
|
#include "modules/StoreForwardModule.h"
|
||||||
#include <Preferences.h>
|
#include <Preferences.h>
|
||||||
#include <esp_efuse.h>
|
|
||||||
#include <esp_efuse_table.h>
|
|
||||||
#include <nvs_flash.h>
|
#include <nvs_flash.h>
|
||||||
#include <soc/efuse_reg.h>
|
|
||||||
#include <soc/soc.h>
|
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
#ifdef ARCH_PORTDUINO
|
#ifdef ARCH_PORTDUINO
|
||||||
@@ -372,8 +369,7 @@ void NodeDB::disarmNodeDatabaseDecodeTargets()
|
|||||||
*/
|
*/
|
||||||
uint32_t radioGeneration;
|
uint32_t radioGeneration;
|
||||||
|
|
||||||
// FIXME - move this somewhere else
|
// getMacAddr() and getDeviceId() are the per-architecture hooks declared in target_specific.h.
|
||||||
extern void getMacAddr(uint8_t *dmac);
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
*
|
*
|
||||||
@@ -410,53 +406,13 @@ NodeDB::NodeDB()
|
|||||||
uint32_t channelFileCRC = crc32Buffer(&channelFile, sizeof(channelFile));
|
uint32_t channelFileCRC = crc32Buffer(&channelFile, sizeof(channelFile));
|
||||||
|
|
||||||
int saveWhat = 0;
|
int saveWhat = 0;
|
||||||
// Get device unique id
|
// Re-read the device id from silicon each boot via the per-arch getDeviceId(); clear the
|
||||||
#if defined(CONFIG_IDF_TARGET_ESP32C3) || defined(CONFIG_IDF_TARGET_ESP32S3) || defined(CONFIG_IDF_TARGET_ESP32C6)
|
// disk-loaded value first so a failed/empty derivation leaves it unset rather than stale.
|
||||||
uint32_t unique_id[4];
|
myNodeInfo.device_id.size = 0;
|
||||||
// ESP32 factory burns a unique id in efuse for S2+ series and evidently C3+ series
|
memset(myNodeInfo.device_id.bytes, 0, sizeof(myNodeInfo.device_id.bytes));
|
||||||
// This is used for HMACs in the esp-rainmaker AIOT platform and seems to be a good choice for us
|
if (getDeviceId(myNodeInfo.device_id.bytes)) {
|
||||||
esp_err_t err = esp_efuse_read_field_blob(ESP_EFUSE_OPTIONAL_UNIQUE_ID, unique_id, sizeof(unique_id) * 8);
|
myNodeInfo.device_id.size = sizeof(myNodeInfo.device_id.bytes);
|
||||||
if (err == ESP_OK) {
|
|
||||||
memcpy(myNodeInfo.device_id.bytes, unique_id, sizeof(unique_id));
|
|
||||||
myNodeInfo.device_id.size = 16;
|
|
||||||
} else {
|
|
||||||
LOG_WARN("Failed to read unique id from efuse");
|
|
||||||
}
|
}
|
||||||
#elif defined(ARCH_NRF54L15)
|
|
||||||
// nRF54L15: DEVICEID is under FICR->INFO sub-struct (not top-level as on nRF52)
|
|
||||||
uint64_t device_id_start = ((uint64_t)NRF_FICR->INFO.DEVICEID[1] << 32) | NRF_FICR->INFO.DEVICEID[0];
|
|
||||||
uint64_t device_id_end = ((uint64_t)NRF_FICR->DEVICEADDR[1] << 32) | NRF_FICR->DEVICEADDR[0];
|
|
||||||
memcpy(myNodeInfo.device_id.bytes, &device_id_start, sizeof(device_id_start));
|
|
||||||
memcpy(myNodeInfo.device_id.bytes + sizeof(device_id_start), &device_id_end, sizeof(device_id_end));
|
|
||||||
myNodeInfo.device_id.size = 16;
|
|
||||||
#elif defined(ARCH_NRF52)
|
|
||||||
// Nordic applies a FIPS compliant Random ID to each chip at the factory
|
|
||||||
// We concatenate the device address to the Random ID to create a unique ID for now
|
|
||||||
// This will likely utilize a crypto module in the future
|
|
||||||
uint64_t device_id_start = ((uint64_t)NRF_FICR->DEVICEID[1] << 32) | NRF_FICR->DEVICEID[0];
|
|
||||||
uint64_t device_id_end = ((uint64_t)NRF_FICR->DEVICEADDR[1] << 32) | NRF_FICR->DEVICEADDR[0];
|
|
||||||
memcpy(myNodeInfo.device_id.bytes, &device_id_start, sizeof(device_id_start));
|
|
||||||
memcpy(myNodeInfo.device_id.bytes + sizeof(device_id_start), &device_id_end, sizeof(device_id_end));
|
|
||||||
myNodeInfo.device_id.size = 16;
|
|
||||||
// Uncomment below to print the device id
|
|
||||||
#elif ARCH_PORTDUINO
|
|
||||||
if (portduino_config.has_device_id) {
|
|
||||||
memcpy(myNodeInfo.device_id.bytes, portduino_config.device_id, 16);
|
|
||||||
myNodeInfo.device_id.size = 16;
|
|
||||||
}
|
|
||||||
#else
|
|
||||||
// FIXME - implement for other platforms
|
|
||||||
#endif
|
|
||||||
|
|
||||||
// if (myNodeInfo.device_id.size == 16) {
|
|
||||||
// std::string deviceIdHex;
|
|
||||||
// for (size_t i = 0; i < myNodeInfo.device_id.size; ++i) {
|
|
||||||
// char buf[3];
|
|
||||||
// snprintf(buf, sizeof(buf), "%02X", myNodeInfo.device_id.bytes[i]);
|
|
||||||
// deviceIdHex += buf;
|
|
||||||
// }
|
|
||||||
// LOG_DEBUG("Device ID (HEX): %s", deviceIdHex.c_str());
|
|
||||||
// }
|
|
||||||
|
|
||||||
// likewise - we always want the app requirements to come from the running appload
|
// likewise - we always want the app requirements to come from the running appload
|
||||||
myNodeInfo.min_app_version = 30200; // format is Mmmss (where M is 1+the numeric major number. i.e. 30200 means 2.2.00
|
myNodeInfo.min_app_version = 30200; // format is Mmmss (where M is 1+the numeric major number. i.e. 30200 means 2.2.00
|
||||||
|
|||||||
@@ -1,4 +1,5 @@
|
|||||||
#include "meshUtils.h"
|
#include "meshUtils.h"
|
||||||
|
#include "target_specific.h"
|
||||||
#include <string.h>
|
#include <string.h>
|
||||||
|
|
||||||
/*
|
/*
|
||||||
@@ -79,6 +80,20 @@ bool memfll(const uint8_t *mem, uint8_t find, size_t numbytes)
|
|||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
bool getMacAddrDeviceId(uint8_t *deviceId)
|
||||||
|
{
|
||||||
|
// Zero-initialized: getMacAddr() may return without writing (e.g. Portduino with no MAC
|
||||||
|
// source), and we want that no-write case to hit the all-zeros guard below, not read garbage.
|
||||||
|
uint8_t mac[6] = {0};
|
||||||
|
getMacAddr(mac);
|
||||||
|
if (memfll(mac, 0, sizeof(mac))) {
|
||||||
|
LOG_WARN("MAC is all zeros, leaving device_id unset");
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
memcpy(deviceId, mac, sizeof(mac));
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
bool isOneOf(int item, int count, ...)
|
bool isOneOf(int item, int count, ...)
|
||||||
{
|
{
|
||||||
va_list args;
|
va_list args;
|
||||||
|
|||||||
@@ -49,6 +49,10 @@ void printBytes(const char *label, const uint8_t *p, size_t numbytes);
|
|||||||
// is the memory region filled with a single character?
|
// is the memory region filled with a single character?
|
||||||
bool memfll(const uint8_t *mem, uint8_t find, size_t numbytes);
|
bool memfll(const uint8_t *mem, uint8_t find, size_t numbytes);
|
||||||
|
|
||||||
|
// getDeviceId() fallback (see target_specific.h): copies the 6-byte factory MAC, or returns false
|
||||||
|
// on an all-zero MAC so two blank devices don't collide on an all-zero id.
|
||||||
|
bool getMacAddrDeviceId(uint8_t *deviceId);
|
||||||
|
|
||||||
bool isOneOf(int item, int count, ...);
|
bool isOneOf(int item, int count, ...);
|
||||||
|
|
||||||
const std::string vformat(const char *const zcFormat, ...);
|
const std::string vformat(const char *const zcFormat, ...);
|
||||||
|
|||||||
@@ -27,6 +27,8 @@
|
|||||||
#include "target_specific.h"
|
#include "target_specific.h"
|
||||||
#include <Preferences.h>
|
#include <Preferences.h>
|
||||||
#include <driver/rtc_io.h>
|
#include <driver/rtc_io.h>
|
||||||
|
#include <esp_efuse.h>
|
||||||
|
#include <esp_efuse_table.h>
|
||||||
#include <nvs.h>
|
#include <nvs.h>
|
||||||
#include <nvs_flash.h>
|
#include <nvs_flash.h>
|
||||||
|
|
||||||
@@ -158,6 +160,26 @@ void getMacAddr(uint8_t *dmac)
|
|||||||
#endif
|
#endif
|
||||||
}
|
}
|
||||||
|
|
||||||
|
bool getDeviceId(uint8_t *deviceId)
|
||||||
|
{
|
||||||
|
#if defined(CONFIG_IDF_TARGET_ESP32C3) || defined(CONFIG_IDF_TARGET_ESP32S2) || defined(CONFIG_IDF_TARGET_ESP32S3) || \
|
||||||
|
defined(CONFIG_IDF_TARGET_ESP32C6)
|
||||||
|
// ESP32 factory burns a 128-bit unique id in efuse for S2+ and C3+ series (used for HMACs
|
||||||
|
// in the esp-rainmaker AIOT platform); a good stable hardware anchor for us too.
|
||||||
|
uint32_t unique_id[4];
|
||||||
|
esp_err_t err = esp_efuse_read_field_blob(ESP_EFUSE_OPTIONAL_UNIQUE_ID, unique_id, sizeof(unique_id) * 8);
|
||||||
|
if (err != ESP_OK) {
|
||||||
|
LOG_WARN("Failed to read unique id from efuse");
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
memcpy(deviceId, unique_id, sizeof(unique_id));
|
||||||
|
return true;
|
||||||
|
#else
|
||||||
|
// Classic ESP32 has no OPTIONAL_UNIQUE_ID efuse: fall back to the factory-burned MAC.
|
||||||
|
return getMacAddrDeviceId(deviceId);
|
||||||
|
#endif
|
||||||
|
}
|
||||||
|
|
||||||
#if HAS_32768HZ
|
#if HAS_32768HZ
|
||||||
#define CALIBRATE_ONE(cali_clk) calibrate_one(cali_clk, #cali_clk)
|
#define CALIBRATE_ONE(cali_clk) calibrate_one(cali_clk, #cali_clk)
|
||||||
|
|
||||||
|
|||||||
@@ -186,6 +186,17 @@ void getMacAddr(uint8_t *dmac)
|
|||||||
dmac[0] = src[5] | 0xc0; // MSB high two bits get set elsewhere in the bluetooth stack
|
dmac[0] = src[5] | 0xc0; // MSB high two bits get set elsewhere in the bluetooth stack
|
||||||
}
|
}
|
||||||
|
|
||||||
|
bool getDeviceId(uint8_t *deviceId)
|
||||||
|
{
|
||||||
|
// Nordic burns a FIPS-compliant random id into each chip at the factory. We concatenate
|
||||||
|
// the device address to that random id to form the 16-byte hardware identifier.
|
||||||
|
uint64_t device_id_start = ((uint64_t)NRF_FICR->DEVICEID[1] << 32) | NRF_FICR->DEVICEID[0];
|
||||||
|
uint64_t device_id_end = ((uint64_t)NRF_FICR->DEVICEADDR[1] << 32) | NRF_FICR->DEVICEADDR[0];
|
||||||
|
memcpy(deviceId, &device_id_start, sizeof(device_id_start));
|
||||||
|
memcpy(deviceId + sizeof(device_id_start), &device_id_end, sizeof(device_id_end));
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
#if !MESHTASTIC_EXCLUDE_BLUETOOTH
|
#if !MESHTASTIC_EXCLUDE_BLUETOOTH
|
||||||
void setBluetoothEnable(bool enable)
|
void setBluetoothEnable(bool enable)
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -12,6 +12,7 @@
|
|||||||
#include <SPI.h>
|
#include <SPI.h>
|
||||||
#include <Wire.h>
|
#include <Wire.h>
|
||||||
#include <assert.h>
|
#include <assert.h>
|
||||||
|
#include <cstring>
|
||||||
#include <stdio.h>
|
#include <stdio.h>
|
||||||
|
|
||||||
#include "NodeDB.h"
|
#include "NodeDB.h"
|
||||||
@@ -97,6 +98,17 @@ void getMacAddr(uint8_t *dmac)
|
|||||||
#endif
|
#endif
|
||||||
}
|
}
|
||||||
|
|
||||||
|
bool getDeviceId(uint8_t *deviceId)
|
||||||
|
{
|
||||||
|
// nRF54L15: DEVICEID under the FICR->INFO sub-struct. Read unconditionally so a future build
|
||||||
|
// lacking NRF_FICR fails loudly rather than silently sharing getMacAddr()'s placeholder MAC.
|
||||||
|
uint64_t device_id_start = ((uint64_t)NRF_FICR->INFO.DEVICEID[1] << 32) | NRF_FICR->INFO.DEVICEID[0];
|
||||||
|
uint64_t device_id_end = ((uint64_t)NRF_FICR->DEVICEADDR[1] << 32) | NRF_FICR->DEVICEADDR[0];
|
||||||
|
memcpy(deviceId, &device_id_start, sizeof(device_id_start));
|
||||||
|
memcpy(deviceId + sizeof(device_id_start), &device_id_end, sizeof(device_id_end));
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
// ── Bluetooth ─────────────────────────────────────────────────────────────────
|
// ── Bluetooth ─────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
void setBluetoothEnable(bool enable)
|
void setBluetoothEnable(bool enable)
|
||||||
|
|||||||
@@ -201,6 +201,16 @@ void getMacAddr(uint8_t *dmac)
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
bool getDeviceId(uint8_t *deviceId)
|
||||||
|
{
|
||||||
|
if (portduino_config.has_device_id) {
|
||||||
|
memcpy(deviceId, portduino_config.device_id, sizeof(portduino_config.device_id));
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
// Config-supplied id stays preferred: host NIC/BT MACs can be unstable (docker, multi-NIC).
|
||||||
|
return getMacAddrDeviceId(deviceId);
|
||||||
|
}
|
||||||
|
|
||||||
std::string cleanupNameForAutoconf(std::string name)
|
std::string cleanupNameForAutoconf(std::string name)
|
||||||
{
|
{
|
||||||
// Convert spaces -> dashes, lowercase
|
// Convert spaces -> dashes, lowercase
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
#include "HardwareRNG.h"
|
#include "HardwareRNG.h"
|
||||||
#include "configuration.h"
|
#include "configuration.h"
|
||||||
#include "hardware/xosc.h"
|
#include "hardware/xosc.h"
|
||||||
|
#include <cstring>
|
||||||
#include <hardware/clocks.h>
|
#include <hardware/clocks.h>
|
||||||
#include <hardware/pll.h>
|
#include <hardware/pll.h>
|
||||||
#include <hardware/watchdog.h>
|
#include <hardware/watchdog.h>
|
||||||
@@ -98,6 +99,15 @@ void getMacAddr(uint8_t *dmac)
|
|||||||
dmac[0] = src.id[2];
|
dmac[0] = src.id[2];
|
||||||
}
|
}
|
||||||
|
|
||||||
|
bool getDeviceId(uint8_t *deviceId)
|
||||||
|
{
|
||||||
|
// RP2040/RP2350: 64-bit unique board id (flash serial / OTP) in bytes 0-7 (rest stay zero).
|
||||||
|
pico_unique_board_id_t board_id;
|
||||||
|
pico_get_unique_board_id(&board_id);
|
||||||
|
memcpy(deviceId, board_id.id, sizeof(board_id.id));
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
void rp2040Setup()
|
void rp2040Setup()
|
||||||
{
|
{
|
||||||
if (watchdog_caused_reboot()) {
|
if (watchdog_caused_reboot()) {
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
#include "RTC.h"
|
#include "RTC.h"
|
||||||
#include "configuration.h"
|
#include "configuration.h"
|
||||||
|
#include <cstring>
|
||||||
#include <stdarg.h>
|
#include <stdarg.h>
|
||||||
#include <stm32wle5xx.h>
|
#include <stm32wle5xx.h>
|
||||||
#include <stm32wlxx_hal.h>
|
#include <stm32wlxx_hal.h>
|
||||||
@@ -77,6 +78,14 @@ void getMacAddr(uint8_t *dmac)
|
|||||||
dmac[0] = (uint8_t)(uid2 >> 8);
|
dmac[0] = (uint8_t)(uid2 >> 8);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
bool getDeviceId(uint8_t *deviceId)
|
||||||
|
{
|
||||||
|
// STM32WL: 96-bit factory silicon UID (words w0..w2, little-endian) in bytes 0-11 (rest stay zero).
|
||||||
|
uint32_t uid[3] = {HAL_GetUIDw0(), HAL_GetUIDw1(), HAL_GetUIDw2()};
|
||||||
|
memcpy(deviceId, uid, sizeof(uid));
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
void cpuDeepSleep(uint32_t msecToWake) {}
|
void cpuDeepSleep(uint32_t msecToWake) {}
|
||||||
|
|
||||||
// Hacks to force more code and data out.
|
// Hacks to force more code and data out.
|
||||||
|
|||||||
@@ -8,3 +8,7 @@
|
|||||||
void setBluetoothEnable(bool enable);
|
void setBluetoothEnable(bool enable);
|
||||||
|
|
||||||
void getMacAddr(uint8_t *dmac);
|
void getMacAddr(uint8_t *dmac);
|
||||||
|
|
||||||
|
// Fill deviceId (a caller-zeroed 16-byte buffer) with a stable silicon/factory id; return true, or
|
||||||
|
// false (buffer untouched) if none exists here. Writes only leading bytes. Per-arch: src/platform/<arch>/.
|
||||||
|
bool getDeviceId(uint8_t *deviceId);
|
||||||
|
|||||||
Reference in New Issue
Block a user