Skip to content

Add counter support feature #50

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Add counter support feature #50

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cbcf894
Update ECCX08.h with method description
maihde Jul 16, 2023
0fe82c9
Update ECCX08.cpp with counter functions
maihde Jul 16, 2023
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
50 changes: 49 additions & 1 deletion src/ECCX08.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,54 @@ int ECCX08Class::nonce(const byte data[])
return challenge(data);
}

long ECCX08Class::incrementCounter(int keyId)
{
uint32_t counter; // the counter can go up to 2,097,151

if (!wakeup()) {
return -1;
}

if (!sendCommand(0x24, 1, keyId)) {
return -1;
}

delay(20);

if (!receiveResponse(&counter, sizeof(counter))) {
return -1;
}

delay(1);
idle();

return counter;
}
Comment on lines +551 to +573
Copy link
Contributor

@pennam pennam Sep 18, 2023

Choose a reason for hiding this comment

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

To keep changes aligned to the style of the library i would split this function in two:

  • one basic function returning 0 or 1 and taking the counter value as argument;
  • another one that wraps it and returns the counter value

Also, reading the docs it looks to me that keyId can be only 0 or 1 so i would add a check to keyId parameter.

Suggested change
long ECCX08Class::incrementCounter(int keyId)
{
uint32_t counter; // the counter can go up to 2,097,151
if (!wakeup()) {
return -1;
}
if (!sendCommand(0x24, 1, keyId)) {
return -1;
}
delay(20);
if (!receiveResponse(&counter, sizeof(counter))) {
return -1;
}
delay(1);
idle();
return counter;
}
int ECCX08Class::incrementCounter(int keyId, long& counter)
{
if (keyId < 0 || keyId > 1) {
return 0;
}
if (!wakeup()) {
return 0;
}
if (!sendCommand(0x24, 1, keyId)) {
return 0;
}
delay(20);
if (!receiveResponse(&counter, sizeof(counter))) {
return 0;
}
delay(1);
idle();
return 1;
}
long ECCX08Class::incrementCounter(int keyId)
{
long counter; // the counter can go up to 2,097,151
if(!incrementCounter(keyId, counter)) {
return -1;
}
return counter;
}


long ECCX08Class::readCounter(int keyId)
{
uint32_t counter; // the counter can go up to 2,097,151

if (!wakeup()) {
return -1;
}

if (!sendCommand(0x24, 0, keyId)) {
return -1;
}

delay(20);

if (!receiveResponse(&counter, sizeof(counter))) {
return -1;
}

delay(1);
idle();

return counter;
}
Copy link
Contributor

@pennam pennam Sep 18, 2023

Choose a reason for hiding this comment

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

I would do the same here


int ECCX08Class::wakeup()
{
_wire->setClock(_wakeupFrequency);
Expand Down Expand Up @@ -892,4 +940,4 @@ uint16_t ECCX08Class::crc16(const byte data[], size_t length)
ECCX08Class ECCX08(CRYPTO_WIRE, 0x60);
#else
ECCX08Class ECCX08(Wire, 0x60);
#endif
#endif
3 changes: 3 additions & 0 deletions src/ECCX08.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ class ECCX08Class

int nonce(const byte data[]);

long incrementCounter(int keyId);
Copy link
Contributor

@pennam pennam Sep 18, 2023

Choose a reason for hiding this comment

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

And update methods declarations accordingly.

long readCounter(int keyId);

private:
int wakeup();
int sleep();
Expand Down