Skip to content

Commit 3e2be65

Browse files
authored
Merge pull request #12 from bofh69/copilot/create-error-enum-for-sketch
Add ErrorCode enum to replace hardcoded negative return values in Arduino sketch
2 parents 12b7492 + beb283b commit 3e2be65

File tree

5 files changed

+137
-33
lines changed

5 files changed

+137
-33
lines changed

.github/workflows/python-ci.yml

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,8 @@ jobs:
5252
pip install black ruff
5353
5454
- name: Check with Black
55-
run: black --check src/ tests/
55+
run: black -l 79 --check src/ tests/
5656

57-
- name: Check with Ruff format
58-
run: ruff format --check src/ tests/
59-
6057
typecheck:
6158
name: Type Check with mypy
6259
runs-on: ubuntu-latest

Makefile

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
# SPDX-FileCopyrightText: 2025 PN5180-tagomatic contributors
2+
# SPDX-License-Identifier: GPL-3.0-or-later
3+
4+
VENV := venv
5+
PYTHON := $(VENV)/bin/python
6+
PIP := $(VENV)/bin/pip
7+
PYTEST := $(VENV)/bin/pytest
8+
RUFF := $(VENV)/bin/ruff
9+
BLACK := $(VENV)/bin/black
10+
MYPY := $(VENV)/bin/mypy
11+
12+
.PHONY: help
13+
help:
14+
@echo "PN5180-tagomatic development targets:"
15+
@echo " make venv - Create virtual environment"
16+
@echo " make install - Install package and dependencies"
17+
@echo " make install-dev - Install package with dev dependencies"
18+
@echo " make test - Run tests"
19+
@echo " make lint - Run linting checks"
20+
@echo " make format - Format code with black"
21+
@echo " make type-check - Run type checking with mypy"
22+
@echo " make clean - Remove virtual environment and build artifacts"
23+
24+
$(VENV)/bin/activate:
25+
python3 -m venv $(VENV)
26+
$(PIP) install --upgrade pip
27+
28+
.PHONY: venv
29+
venv: $(VENV)/bin/activate
30+
31+
# Install package with regular dependencies
32+
$(VENV)/.timestamp: $(VENV)/bin/activate pyproject.toml
33+
$(PIP) install -e .
34+
touch $@
35+
36+
.PHONY: install
37+
install: $(VENV)/.timestamp
38+
39+
# Install package with development dependencies
40+
$(VENV)/.timestamp-dev: $(VENV)/bin/activate pyproject.toml
41+
$(PIP) install -e ".[dev]"
42+
touch $@
43+
44+
.PHONY: install-dev
45+
install-dev: $(VENV)/.timestamp-dev
46+
47+
.PHONY: test
48+
test: install-dev
49+
$(PYTEST)
50+
51+
.PHONY: lint
52+
lint: install-dev
53+
$(RUFF) check src/ tests/
54+
55+
.PHONY: format
56+
format: install-dev
57+
$(BLACK) -l79 src/ tests/
58+
59+
.PHONY: type-check
60+
type-check: install-dev
61+
$(MYPY) src/
62+
63+
.PHONY: clean
64+
clean:
65+
rm -rf $(VENV)
66+
rm -rf build/
67+
rm -rf dist/
68+
rm -rf *.egg-info
69+
rm -rf .pytest_cache
70+
rm -rf .ruff_cache
71+
rm -rf .mypy_cache
72+
rm -rf htmlcov/
73+
rm -rf .coverage
74+
find . -type d -name __pycache__ -exec rm -rf {} + 2>/dev/null || true
75+
find . -type f -name "*.pyc" -delete

README.md

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ SPDX-FileCopyrightText: 2025 PN5180-tagomatic contributors
55
SPDX-License-Identifier: GPL-3.0-or-later
66
-->
77

8-
This is a work in progress. Few things described here actually works
9-
yet.
8+
**This is a work in progress. Some things described here doesn't work yet,
9+
and bug reports are not welcome at this time.**
1010

1111
USB based RFID reader with Python interface
1212

@@ -102,14 +102,13 @@ pytest
102102

103103
```bash
104104
# Linting
105-
ruff check src/ tests/
105+
make check
106106

107107
# Formatting
108-
black src/ tests/
109-
ruff format src/ tests/
108+
make format
110109

111110
# Type checking
112-
mypy src/
111+
make type-check
113112
```
114113

115114
## License
@@ -129,4 +128,5 @@ Contributions are welcome! Please ensure that:
129128

130129
## Acknowledgments
131130

132-
This project uses the PN5180 NFC Frontend from NXP Semiconductors.
131+
This project uses FastLED by Daniel Garcia et al.
132+
It also uses SimpleRPC by Jeroen F.J. Laros, Chris Flesher et al.

sketch/pn5180_reader/pn5180_reader.ino

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,25 @@
2121
#include <FastLED.h>
2222
#include <simpleRPC.h>
2323

24+
// Error codes for negative return values
25+
enum ErrorCode {
26+
ERR_DATA_LEN_TOO_LARGE = -1,
27+
ERR_SEND_BUSY_CHECK_FIRST = -2,
28+
ERR_SEND_BUSY_CHECK_SECOND = -3,
29+
ERR_SEND_BUSY_CHECK_THIRD = -4,
30+
ERR_RECV_BUSY_CHECK_FIRST = -5,
31+
ERR_RECV_BUSY_CHECK_SECOND = -6,
32+
ERR_RECV_BUSY_CHECK_THIRD = -7,
33+
ERR_TOO_MANY_ELEMENTS = -8,
34+
ERR_TOO_MANY_ADDRESSES = -9,
35+
ERR_EEPROM_DATA_TOO_LARGE = -10,
36+
ERR_LEN_TOO_LARGE = -11,
37+
ERR_TOO_MANY_PARAMS = -12,
38+
ERR_SELECT_COMMAND_TOO_LARGE = -13,
39+
ERR_TX_DATA_TOO_LARGE = -14,
40+
ERR_SEND_DATA_TOO_LARGE = -15
41+
};
42+
2443
// SPI commands for PN5180:
2544
static const uint8_t PN5180_WRITE_REGISTER = 0x00;
2645
static const uint8_t PN5180_WRITE_REGISTER_OR_MASK = 0x01;
@@ -92,13 +111,13 @@ static int send_spi_data(const uint8_t* data, size_t data_len) {
92111
uint8_t buffer[256];
93112
if (data_len > sizeof(buffer)) {
94113
log("data_len too large");
95-
return -1;
114+
return ERR_DATA_LEN_TOO_LARGE;
96115
}
97116
memcpy(buffer, data, data_len);
98117

99118
if (!wait_busy_is(LOW)) {
100119
log("First busy check failed");
101-
return -2;
120+
return ERR_SEND_BUSY_CHECK_FIRST;
102121
}
103122

104123
digitalWrite(PN5180_NSS, LOW);
@@ -110,15 +129,15 @@ static int send_spi_data(const uint8_t* data, size_t data_len) {
110129
int retval = 0;
111130
if (!wait_busy_is(HIGH)) {
112131
log("Second busy check failed");
113-
retval = -3;
132+
retval = ERR_SEND_BUSY_CHECK_SECOND;
114133
}
115134

116135
PN_SPI.endTransaction();
117136
digitalWrite(PN5180_NSS, HIGH);
118137

119138
if (!retval && !wait_busy_is(LOW)) {
120139
log("Third busy check failed");
121-
retval = -4;
140+
retval = ERR_SEND_BUSY_CHECK_THIRD;
122141
}
123142

124143
return retval;
@@ -129,7 +148,7 @@ static int recv_spi_data(uint8_t* buffer, size_t buffer_len) {
129148

130149
if (!wait_busy_is(LOW)) {
131150
log("First busy check failed");
132-
return -1;
151+
return ERR_RECV_BUSY_CHECK_FIRST;
133152
}
134153

135154
digitalWrite(PN5180_NSS, LOW);
@@ -141,15 +160,15 @@ static int recv_spi_data(uint8_t* buffer, size_t buffer_len) {
141160
int retval = 0;
142161
if (!wait_busy_is(HIGH)) {
143162
log("Second busy check failed");
144-
retval = -2;
163+
retval = ERR_RECV_BUSY_CHECK_SECOND;
145164
}
146165

147166
PN_SPI.endTransaction();
148167
digitalWrite(PN5180_NSS, HIGH);
149168

150169
if (!retval && !wait_busy_is(LOW)) {
151170
log("Third busy check failed");
152-
retval = -3;
171+
retval = ERR_RECV_BUSY_CHECK_THIRD;
153172
}
154173

155174
return retval;
@@ -229,7 +248,7 @@ static int write_register_and_mask(uint8_t addr, uint32_t value) {
229248
static int write_register_multiple(Vector<Object<uint8_t, uint8_t, uint32_t>>& elements) {
230249
if (elements.size > 42) {
231250
log("Too many elements");
232-
return -1;
251+
return ERR_TOO_MANY_ELEMENTS;
233252
}
234253

235254
uint8_t buffer[211];
@@ -290,7 +309,7 @@ static Object<int, Vector<uint32_t>> read_register_multiple(Vector<uint8_t>& add
290309

291310
if (addrs.size > 18) {
292311
log("Too many addresses");
293-
get<0>(result) = -1;
312+
get<0>(result) = ERR_TOO_MANY_ADDRESSES;
294313
return result;
295314
}
296315

@@ -330,7 +349,7 @@ static int16_t write_eeprom(uint8_t addr, Vector<uint8_t>& values) {
330349
uint8_t buffer[256];
331350
if (values.size > 255) {
332351
log("Too much data to write");
333-
return -1;
352+
return ERR_EEPROM_DATA_TOO_LARGE;
334353
}
335354

336355
buffer[0] = PN5180_WRITE_EEPROM;
@@ -385,7 +404,7 @@ static int16_t write_tx_data(Vector<uint8_t>& values) {
385404
uint8_t buffer[261];
386405
if (values.size > 260) {
387406
log("Too much data to write");
388-
return -1;
407+
return ERR_TX_DATA_TOO_LARGE;
389408
}
390409

391410
buffer[0] = PN5180_WRITE_TX_DATA;
@@ -413,7 +432,7 @@ static int16_t send_data(uint8_t bits, Vector<uint8_t>& values) {
413432
uint8_t buffer[262];
414433
if (values.size > 260) {
415434
log("Too much data to write");
416-
return -1;
435+
return ERR_SEND_DATA_TOO_LARGE;
417436
}
418437

419438
buffer[0] = PN5180_SEND_DATA;
@@ -442,7 +461,7 @@ static Object<int, Vector<uint8_t>> read_data(uint16_t len) {
442461

443462
if (len > sizeof(response)) {
444463
log("len too large");
445-
get<0>(result) = -1;
464+
get<0>(result) = ERR_LEN_TOO_LARGE;
446465
return result;
447466
}
448467

@@ -486,7 +505,7 @@ static int switch_mode(uint8_t mode, Vector<uint8_t>& params) {
486505
uint8_t buffer[1 + 1 + 4];
487506
if (params.size > 4) {
488507
log("Too many params");
489-
return -1;
508+
return ERR_TOO_MANY_PARAMS;
490509
}
491510
buffer[0] = PN5180_SWITCH_MODE;
492511
buffer[1] = mode;
@@ -553,7 +572,7 @@ static int16_t epc_inventory(Vector<uint8_t>& select_command, uint8_t select_com
553572
uint8_t begin_round[3], uint8_t timeslot_behavior) {
554573
if (select_command.size > 39) {
555574
log("Too large select_command");
556-
return -1;
575+
return ERR_SELECT_COMMAND_TOO_LARGE;
557576
}
558577

559578
uint8_t buffer[47];

src/pn5180_tagomatic/pn5180.py

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -201,11 +201,15 @@ def write_register_and_mask(self, addr: int, value: int) -> None:
201201
"""
202202
self._validate_uint8(addr, "addr")
203203
self._validate_uint32(value, "value")
204-
result = cast(int, self._interface.write_register_and_mask(addr, value))
204+
result = cast(
205+
int, self._interface.write_register_and_mask(addr, value)
206+
)
205207
if result < 0:
206208
raise PN5180Error("write_register_and_mask", result)
207209

208-
def write_register_multiple(self, elements: List[Tuple[int, int, int]]) -> None:
210+
def write_register_multiple(
211+
self, elements: List[Tuple[int, int, int]]
212+
) -> None:
209213
"""Write to multiple PN5180 registers.
210214
211215
Args:
@@ -268,7 +272,8 @@ def read_register_multiple(self, addrs: List[int]) -> List[int]:
268272
for i, addr in enumerate(addrs):
269273
self._validate_uint8(addr, f"addrs[{i}]")
270274
result = cast(
271-
Tuple[int, List[int]], self._interface.read_register_multiple(addrs)
275+
Tuple[int, List[int]],
276+
self._interface.read_register_multiple(addrs),
272277
)
273278
if result[0] < 0:
274279
raise PN5180Error("read_register_multiple", result[0])
@@ -416,7 +421,9 @@ def mifare_authenticate(
416421
self._validate_uint32(uid, "uid")
417422
result = cast(
418423
int,
419-
self._interface.mifare_authenticate(list(key), key_type, block_addr, uid),
424+
self._interface.mifare_authenticate(
425+
list(key), key_type, block_addr, uid
426+
),
420427
)
421428
if result < 0:
422429
raise PN5180Error("mifare_authenticate", result)
@@ -445,7 +452,9 @@ def epc_inventory(
445452
"""
446453
if len(select_command) > 39:
447454
raise ValueError("select_command must be at most 39 bytes")
448-
self._validate_uint8(select_command_final_bits, "select_command_final_bits")
455+
self._validate_uint8(
456+
select_command_final_bits, "select_command_final_bits"
457+
)
449458
if len(begin_round) != 3:
450459
raise ValueError("begin_round must be exactly 3 bytes")
451460
if timeslot_behavior not in (
@@ -489,7 +498,9 @@ def epc_retrieve_inventory_result_size(self) -> int:
489498
Raises:
490499
PN5180Error: If the operation fails.
491500
"""
492-
result = cast(int, self._interface.epc_retrieve_inventory_result_size())
501+
result = cast(
502+
int, self._interface.epc_retrieve_inventory_result_size()
503+
)
493504
if result < 0:
494505
raise PN5180Error("epc_retrieve_inventory_result_size", result)
495506
return result
@@ -506,7 +517,9 @@ def load_rf_config(self, tx_config: int, rx_config: int) -> None:
506517
"""
507518
self._validate_uint8(tx_config, "tx_config")
508519
self._validate_uint8(rx_config, "rx_config")
509-
result = cast(int, self._interface.load_rf_config(tx_config, rx_config))
520+
result = cast(
521+
int, self._interface.load_rf_config(tx_config, rx_config)
522+
)
510523
if result < 0:
511524
raise PN5180Error("load_rf_config", result)
512525

0 commit comments

Comments
 (0)