New way to create unique serial number#230
New way to create unique serial number#230MobiFlight-Admin merged 10 commits intoMobiFlight:mainfrom
Conversation
|
Firmware for this pull request: |
| #include "Button.h" | ||
| #include "Encoder.h" | ||
| #include "MFEEPROM.h" | ||
| #include "ArduinoUniqueID.h" |
There was a problem hiding this comment.
I don't think this needs to be included in mobiflight.cpp since UniqueId isn't used anywhere in the file.
There was a problem hiding this comment.
Oh yes, that's an fragment from my testing.
src/Config.cpp
Outdated
| if (!force && MFeeprom.read_byte(MEM_OFFSET_SERIAL) == 'I' && MFeeprom.read_byte(MEM_OFFSET_SERIAL + 1) == 'D') { | ||
| sprintf(serial, "SN-"); | ||
| for (size_t i = 0; i < UniqueIDsize; i++) { | ||
| if (UniqueID[i] < 0x10) { | ||
| sprintf(&serial[3 + i * 2], "0%X", UniqueID[i]); | ||
| } else { | ||
| sprintf(&serial[3 + i * 2], "%X", UniqueID[i]); | ||
| } | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| // Coming here it's the first start up of the board and no serial number or UniqueID is available | ||
| // Read the uniqueID and use it as serial numnber | ||
| if (!force) { | ||
| sprintf(serial, "SN-"); | ||
| for (size_t i = 0; i < UniqueIDsize; i++) { | ||
| if (UniqueID[i] < 0x10) { | ||
| sprintf(&serial[3 + i * 2], "0%X", UniqueID[i]); | ||
| } else { | ||
| sprintf(&serial[3 + i * 2], "%X", UniqueID[i]); | ||
| } | ||
| } | ||
| // and mark this in the eeprom | ||
| MFeeprom.write_block(MEM_OFFSET_SERIAL, "ID", 2); | ||
| // Set first byte of config to 0x00 to ensure with empty config on 1st start up | ||
| MFeeprom.write_byte(MEM_OFFSET_CONFIG, 0x00); |
There was a problem hiding this comment.
The only difference between these two blocks is whether the word "ID" gets written to eeprom. There's no real downside to writing "ID" every time this is called so this could be simplified significantly and an entire if statement could be removed.
Combined with my prior comment about force being tested first I believe that would simplify this function down to three cases:
- force is true
- SN is in eeprom so there's an old-style ID on the board
- all other cases
There was a problem hiding this comment.
Let me think about this. I will avoid writing to the EEPROM every time the board is started up (so just for reading the EEPROM).
There was a problem hiding this comment.
I think the function can simplify down to something like this:
if (!force && legacy value stored)
{
read legacy value;
return;
}
read unique ID;
if (unique ID marker isn't stored)
{
store ID marker in eeprom;
}I'm pretty sure this covers all the cases. It will only write ID once, and will allow someone to "generate serial" from the desktop to get a new-style ID.
src/Config.cpp
Outdated
| if (!force && serial[0] == 'S' && serial[1] == 'N') | ||
| // A serial number according old style is already generated and saved to the eeprom | ||
| // So keep it to avoid a connector message with orphaned board | ||
| if (!force && MFeeprom.read_byte(MEM_OFFSET_SERIAL) == 'S' && MFeeprom.read_byte(MEM_OFFSET_SERIAL + 1) == 'N') { |
There was a problem hiding this comment.
All of the if statements in this method start with !force, which should get refactored out into a standalone if (force) test at the start of the function to do whatever force does (which as far as I can tell is nothing?). This will make for easier to follow function logic and simpler conditions on the rest of the if statements.
|
Firmware for this pull request: |
|
I still have to test the ProMicro and how to handle the UniqueID if it works (not listed as processor w/o UniqueID). |
neilenns
left a comment
neilenns
left a comment
There was a problem hiding this comment.
Love the updated version!
DocMoebiuz
left a comment
DocMoebiuz
left a comment
There was a problem hiding this comment.
Nice... this will hopefully help with this weird problem.
|
Firmware for this pull request: |
|
Firmware for this pull request: |
4 participants
@DocMoebiuz and @neilenns found a library with an undocumented feature to read an UniqueID on Atmega2560 and ATmega328. For new boards on first start up this UniqueID is used. In the library is also stated, that it might be possible that
two or more could have the same UniqueID (https://github.com/ricaun/ArduinoUniqueID/blob/master/MCU.md#Disclaimer).
My tests have shown that my megas have different UniqueID's.
But as this can not be excluded, generating a serial number as command from the connector is still available. In this case the random generator is used as before but the starting point are the milliseconds from starting up the board. It is very! unlikely to generate the serial number on exactly the same milliseconds between different boards, so a double serial number should not be generates anymore.
On startup of the board it is checked if a serial number was always generated. This is marked in the EEPROM with "SN" on the first two memory locations. In this case this serial number will still be used. Nothing changes for the user.
If a serial number is not available, it is checked if "ID" was written to the EEPROM. This means it is NOT the first start up and the UniqueID will be used as serial number.
If "SN" and "ID" is not read from the EEPROM, it is a first start up of the board an the UniqueID is used as serial number. This is also marked with writing "ID" to the first two EEPROM locations from the serial number locations.
If the user actively request a new serial number, a serial number according the existing style is generated and marked in the EEPROM with "SN". The starting point for the random generator is NOT anymore the analog input A0, it is the time difference from starting up the board until the command to generate a new serial (simply millis()).
In case a UniqueID is used, the serial number is longer than before. The array size for storing the serial number is calculated accordingly for the max. numbers of characters. The original definition
MEM_LEN_SERIALis kept, otherwise the device configuration would start at another position in the EEPROM.Fixes #229