Repository navigation
OpenTag3D: fall back to the bed temperature range when the target is … - #28
Open
ccatlett1984 wants to merge 6 commits into
Open
ccatlett1984 wants to merge 6 commits into
ccatlett1984 wants to merge 6 commits into
Conversation
…unset
# OpenTag3D: fall back to the bed temperature range when the target is unset
## Summary
`bed_temp_c` is read from the tag's bed target only. A tag that populates the min/max pair
but leaves the target at 0 reports a bed temperature of 0 rather than a usable value.
This mirrors the handling already applied to the hotend a few lines above:
```python
hotend_min_temp_c = data["min_print_temp"] or data["print_temp"]
hotend_max_temp_c = data["max_print_temp"] or data["print_temp"]
```
so the bed becomes:
```python
bed_temp_c=data["bed_temp"] or data["min_bed_temp"] or data["max_bed_temp"],
```
One line, plus a regression test.
## Why
The spec carries `bed_temp` alongside `min_bed_temp` and `max_bed_temp`, and nothing
requires a writer to fill in all three. `GenericFilament` has a single bed value, so the
adapter already has to choose — and it chooses the range over the target for the hotend.
Doing the same for the bed keeps one policy rather than two.
Downstream this is the difference between a printer receiving a bed temperature and
receiving nothing: the Snapmaker U1 integration drops non-positive temperatures from its
`filament_detect` payload, so a 0 is indistinguishable from absent.
## Testing
```
$ pytest
57 passed, 1 skipped
```
New test, using the bundled Polar Filament fixture, checks target → min → max precedence
and that all three unset still yields 0:
```python
def test_bed_temp_falls_back_to_range(processor, scan, payload):
payload[148] = 0
payload[149:151] = bytes([11, 13])
assert processor.process_tag(scan, tag(record(payload))).bed_temp_c == 55
payload[149] = 0
assert processor.process_tag(scan, tag(record(payload))).bed_temp_c == 65
payload[150] = 0
assert processor.process_tag(scan, tag(record(payload))).bed_temp_c == 0
```
# fm175xx: read NTAG213 and NTAG216 instead of assuming NTAG215 ## Summary `__reader_a_ultralight_read_all_data` is hard-coded to NTAG215's geometry. It loops to `FM175XX_NTAG215_TOTAL_PAGES` and allocates `FM175XX_NTAG215_TOTAL_SIZE`, so: - **NTAG216 is silently truncated** to 540 of its 924 bytes. An NDEF record living in the upper 384 bytes is invisible, with no error to say so. - **NTAG213 fails outright.** Reads past page 44 are NAKed, and the recovery branch cannot fire (see below), so the whole read returns `FM175XX_CARD_READ_ERR`. It now reads until the tag stops answering and truncates to the largest recognised page count that was fully covered — 180 bytes for an NTAG213, 540 for an NTAG215, 924 for an NTAG216. ## The recovery branch was dead code ```python if (page_no - 4) in Constants.FM175XX_ULTRALIGHT_VALID_END_PAGES: ``` `FM175XX_ULTRALIGHT_VALID_END_PAGES` is `[135, 44]`, and `page_no` comes from `range(0, 135, 4)`, so it takes values `0, 4, … 132` and `page_no - 4` ranges over `-4 … 128`. It can never equal 135 (135 + 4 is not a multiple of 4) and never reaches 44 in a failing iteration, because the loop stops at 132 before a 44-page tag's first NAK at page 48 would be attempted. So any tag that NAKs mid-read falls through to `FM175XX_CARD_READ_ERR`, and any tag larger than an NTAG215 is quietly cut short. This replaces the constant with `FM175XX_ULTRALIGHT_KNOWN_PAGE_COUNTS` and a check that can actually fire. ## Why truncate rather than trust the read length A READ returns four pages and rolls over within addressable memory, so the last successful read on any tag overruns the final page. Truncating to a known page count discards those roll-over bytes, which would otherwise be handed back as if they were tag memory. Plain Ultralight is deliberately **not** in the recognised list, so it stays rejected as it was before. An Ultralight EV1 MF0UL21 has 41 pages and, because of roll-over, answers every read a 44-page tag would; accepting 44 would return 176 bytes whose tail is roll-over rather than memory. `FM175XX_ULTRALIGHT_TOTAL_PAGES` is left in place but is no longer referenced — happy to remove it if you would rather not keep an unused constant. ## Testing ``` $ pytest 66 passed, 1 skipped ``` New `test/test_fm175xx_ultralight_read.py` drives the real function with the page-read helper replaced by a simulated tag that models both behaviours that matter — a READ returns four pages and rolls over within addressable memory, and an address past the last page is NAKed. No hardware is constructed or touched. It asserts that an NTAG213, NTAG215 and NTAG216 each come back at their exact size **and byte-identical to the simulated tag's memory** — the roll-over check, since a naive implementation returns the right length with the wrong tail. It also asserts that 0-, 4-, 16-, 20-, 41- and 44-page tags are still rejected, 41 included specifically because it is a real size that roll-over makes look like 44. ## Scope Only the Ultralight/NTAG path changes. `__reader_a_m1_read_all_data` and the Mifare Classic path are untouched, as is every caller — `read_mifare_ultralight` still receives a byte list and simply gets a correctly sized one.
read NTAG213 and NTAG216 instead of assuming NTAG215
Support ntag216
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OpenTag3D: fall back to the bed temperature range when the target is unset
Summary
bed_temp_cis read from the tag's bed target only. A tag that populates the min/max pair but leaves the target at 0 reports a bed temperature of 0 rather than a usable value.This mirrors the handling already applied to the hotend a few lines above:
so the bed becomes:
One line, plus a regression test.
Why
The spec carries
bed_tempalongsidemin_bed_tempandmax_bed_temp, and nothing requires a writer to fill in all three.GenericFilamenthas a single bed value, so the adapter already has to choose — and it chooses the range over the target for the hotend. Doing the same for the bed keeps one policy rather than two.Downstream this is the difference between a printer receiving a bed temperature and receiving nothing: the Snapmaker U1 integration drops non-positive temperatures from its
filament_detectpayload, so a 0 is indistinguishable from absent.Testing
New test, using the bundled Polar Filament fixture, checks target → min → max precedence and that all three unset still yields 0: