From e2953f02c5e4d0642999ca2e44817fb743b19859 Mon Sep 17 00:00:00 2001 From: Richard Date: Fri, 26 Jun 2026 20:51:12 -0700 Subject: [PATCH] feat(catalog): one sourceless object image per object + on-device download ADR 0018: store and display exactly one survey JPEG per object, named without a survey suffix (/.jpg). Survey provenance becomes a recorded-but-not-runtime-branched curation directive in object_images.source. - New shared core object_image_store.py: on-disk/CDN layout, sourceless resolve_image_name, worklist_for_scope (All/catalog/filter/list) and 404-tolerant download_object_image. Collapses path logic that was duplicated across cat_images / get_images / gen_images. - cat_images: sourceless resolution; get_display_image drops the source concept (orientation/overlay logic unchanged, ADR 0003). - object_details: remove the dead DM_SDSS display mode (never assigned). - object_images gains a nullable source TEXT column (schema + committed DB via ALTER); the importer leaves it NULL (uncurated). gen_images reads it (NULL->POSS) and publishes one sourceless image per object. - On-device Download feature: app-owned ObjectImageDownloader (background worker thread + controller), a Tools > Download Images scope picker (All / Current Filter / Observing List), a preflight (WiFi-client gate, missing count + size estimate + CDN reachability), a progress screen, and a global title-bar status line shown while a download runs. - get_images: rewritten as a thin CLI over the core (All/single-catalog). Retire audit_images.py (dev-only, hardcoded path). - v2.7.0 migration renames legacy _POSS/_SDSS files to the sourceless name (POSS wins when both exist); idempotent and version-gated. - Tests for the core, the controller and the migration; user docs updated for the on-device flow. New UI strings are wrapped in _(); .po extraction is deferred to the project's controlled release-time babel pass (per repo i18n practice). Co-Authored-By: Claude Opus 4.8 (1M context) --- CONTEXT-MAP.md | 2 +- astro_data/pifinder_objects.db | Bin 60157952 -> 60157952 bytes docs/adr/0018-one-object-image-per-object.md | 21 + docs/ax/catalog/CONTEXT.md | 30 ++ docs/source/menu_map.rst | 5 + docs/source/software.rst | 25 +- migration_source/v2.7.0.sh | 6 + pifinder_post_update.sh | 8 + python/PiFinder/audit_images.py | 93 ---- python/PiFinder/cat_images.py | 51 +-- .../catalog_imports/catalog_import_utils.py | 8 +- python/PiFinder/db/objects_db.py | 7 + python/PiFinder/gen_images.py | 182 ++++---- python/PiFinder/get_images.py | 234 ++++------ python/PiFinder/main.py | 7 + .../migrations/v2_7_0_sourceless_images.py | 92 ++++ python/PiFinder/object_image_download.py | 186 ++++++++ python/PiFinder/object_image_store.py | 296 +++++++++++++ python/PiFinder/ui/base.py | 45 ++ python/PiFinder/ui/image_download.py | 412 ++++++++++++++++++ python/PiFinder/ui/menu_manager.py | 7 + python/PiFinder/ui/menu_structure.py | 23 + python/PiFinder/ui/object_details.py | 13 +- .../tests/test_migration_sourceless_images.py | 76 ++++ python/tests/test_object_image_download.py | 120 +++++ python/tests/test_object_image_store.py | 210 +++++++++ python/tests/test_ui_modules.py | 13 +- 27 files changed, 1757 insertions(+), 415 deletions(-) create mode 100644 docs/adr/0018-one-object-image-per-object.md create mode 100644 migration_source/v2.7.0.sh delete mode 100644 python/PiFinder/audit_images.py create mode 100644 python/PiFinder/migrations/v2_7_0_sourceless_images.py create mode 100644 python/PiFinder/object_image_download.py create mode 100644 python/PiFinder/object_image_store.py create mode 100644 python/PiFinder/ui/image_download.py create mode 100644 python/tests/test_migration_sourceless_images.py create mode 100644 python/tests/test_object_image_download.py create mode 100644 python/tests/test_object_image_store.py diff --git a/CONTEXT-MAP.md b/CONTEXT-MAP.md index 1f77e97b7..4662cbfb8 100644 --- a/CONTEXT-MAP.md +++ b/CONTEXT-MAP.md @@ -20,7 +20,7 @@ PiFinder is a multi-process Raspberry Pi finder/plate-solver. These contexts eac - **SQM → Camera**: `shared_state.set_noise_floor()` feeds the minimum acceptable background used by the Camera context's background controller. - **Positioning → Camera**: `Matches` is published on every solve attempt (success or failure) as the feedback signal for solver-driven auto-exposure. - **Catalog ↔ Positioning**: Catalog supplies the `(RA, Dec)` target for the alignment flow that calibrates `solve_pixel` in Positioning. -- **Equipment → Catalog**: the active telescope's flip/flop flags and the active eyepiece's true field of view orient and scale the POSS/SDSS object image in `cat_images.get_display_image`. +- **Equipment → Catalog**: the active telescope's flip/flop flags and the active eyepiece's true field of view orient and scale the **object image** (one sourceless survey JPEG per object) in `cat_images.get_display_image`. - **Positioning → Equipment**: the object-image baseline rotation combines the active telescope's flip/flop with the live solve **roll** from `shared_state` (see [ADR 0003](./docs/adr/0003-object-image-orientation.md)). - **Battery → UI**: STATUS (and web/API) display `BatteryState` from `shared_state.battery()` — *consumption is future work; this run is plumbing + tests only*. - **Battery → system-wide**: `hardware_detect` probes the I²C bus at startup and publishes `HardwareCapabilities` into `shared_state`; the battery monitor process only runs when `has_bq25895` is detected (rev-4). The same capabilities record is the source of truth for other rev-dependent decisions. diff --git a/astro_data/pifinder_objects.db b/astro_data/pifinder_objects.db index 40efa599bfa70ca84e88f296b659a07b38e658cc..d3773f269c47a95a630e1e6cb9b34e3209595184 100644 GIT binary patch delta 4456 zcmZwHS0EN_7>Dtfw-7I-MUgZ$B&DUHsUZz*EtPhONYNrHm4vkS-cv(cd+)us_TIbi ze|7vFT)*c&xKE!u_Wm{JSe4>r?`tr42N?{8bc4a@XfT-jrMi2$_!>NtzS)|>3^}Y$ z8tsDYHrx7}vP@AnNhY5hSyl@J&Yv=i&WyD`VwP_9zfXo|N}$QiI?mDB(!tHmJU%4t zfmMcuYeZ;dcyN4bj)c@4X3AU{RW_AfStv_orP#4nHp--Im7U6|aw&V2Tjf!CRX&wp zIVeX}KowL@%2^dsE~>C{RYg=$RZJCEB~(e}rb?;Os*EbD+*LW{p~@>yRY6r$UaFF+ ztg5K0s+#gv)m05uQ`J&FsZ*FGzG|R+RYTQCHC9cOpK7Z7RWsFGwNNcpfNG^$ zt2U~wYNy((4k}P}RGn03)kSqxL8_bTu6n3o)l>CSy;UF8SM^i<)c`e6g{VO)RE4R* zDqKaVAu3W0Rl`)2idMtb2sKiTQlnLjidAD&oQhXt)i^a?C8!B%qMD=<)nt{Vrl@3< zqEgjVm8Pbt>1u|Wsb;C!YL1$#=BfE=fm)~*sl{rETB??*Z})~WSs zgW9Mzsm&@~ZBbj*Hnm-4s2ysj+NE}@J!-Gor!v)kbwC|dhty$pL>*Pf)Nyq}om8jP zX>~@ORp-=sbwOQJm(*o-MO{_b)OB@3-Bh>KZFNW8Rrl0=^*}vTkJMw8rJks#>X~}3 zUZ|Jqm3pn-sJH5!dapjHkLr{9tiGtP>YMtmeyE@7m-?;#sK1H&`^gH-!3fzPJ6M1v zSi!$#YbF~ofi2iUPRIrJkQ?$qUdRXe!2ukh02Bl#aE3zQ0)@d9ia=2)2F0NSlms^@ z1*M@3lm&Mv2Odx!JfQ+q1TUxrm7xk$g=*jp)u9H|gj(POwV@8wg?dmQ8h|e}ghtR9 znt&fP1%GG;&7lRfgaBv-t)UIHg?7*$IzS+Fgig>Ix5V ztc7*39yY*6*aVv)9k#$$*aq7n19rep*af>`5A20~kO}+Y033uva2SrjQ8)(2;RKw7 zQ*av2z*#s4=ivfegiCN4uE15e2G`*R+=N?j8}7hexCi&)0X&39@EEe-2|R^o@El&i zOLzsZ;SIcnckmuQz(@E5pWzF9g>Ud3e!x%o1;61B{58}0GuL1S=3sOwuJ4-LQ<8bTvz3{Aif znu0$xgXYizT0#J{g4WOm+Cn>M4;>&7IzlJt3|*ir1VK0G4m}_kdO|Pg4Sk?5^n?B| z00u$`41!PygTW9E5ikTIVJHlPD2RsPFak!xC>RYf5DQ}<4&q@fjDzuz025#$OoBw1 z3`sBrk|70BVJf7-G?)%EU?$9h*)Rv@!aSG{3t%BEg2k`|mclYv4l7_Ktb*0B2G+tl zSPvUuBW!}rkPcg5D{O=9kO4bjC+vdVum|?SKFEaqZ~zX%Avg?2;3yn}<8T5_!YMcn zXW%THgY$3!F2W_a3|HVPT!ZUy18%}CxD9vUF5H9r@BkjdBX|s1@C2U1Gk6X!;3d3* z*YF13!aH~mAK)W=g3s^;zQQ;74nN>0{DR-`2mYGt{24WvfjJl<8)OFyummgkw`|R1 z117KqJID#Sz#ei#9>@#%AU`;OBNTvw-~`T42wb2rxIz&q3dNu}lz@`p2Bn}hl!3C~ z4&}fD%7Z6VfQsM+m7p?IfvQjqyrDYOfSOPXe4sYefx1u+>O%wYg@(`w8bcHCgQnmQ z&7e87fR+#dt)Mlufws^N+Cv8jgpSY&Izt!e3PI2fxDtm2;_l__Yw(DOx3La4SeR!S zZ9{Fh*aTQ-TSr-%I>I1y2<}O8J@0-LQE{9tt`##-P}xLf>N)T zXT+pg#HCr7C{txrIaE%SOPMKi#g3)2Qr5~w*{a+skFryFRX&wp6;K70y>d{6RAJ?) zoKz9ztXx!4RZO|6;;MuysY)p~Ra%u%WmP%luF5M9RY7^GimHGVvno3g1Dn(6KscMFr zsb;C!DoxE%bJaXGUoB7z)grZ6Em2F=GPPW-P%G6cwOXxFYt=fnUTsht)h4xBZBglJ ztJgX)kvtd6Lo>XsI%&vIYB<@*VPSmQ{7Ux)g5(L-Bb6~1NBfnQjb-(dZM1HXX?3npa}{K z-l})%z51X&s!!^(`l7z7Z|b}Hp?<1g>bLr%{>JO?Cn+!mBjkXbkPFPf9R4j^GFgE& z*nlnMhCE;gc_AO72^FCdctK^T0#%_JR0nUU0X3l()CM1@19hPu)Q1M(3x3cL8bM?5hbGVz0-zZ* zhZfKh0-+TIL2C$xHqaK@K?t;m4$u)gL1zesF3=UaL3ii@J)sx$hA`*@eW4%phXF7U z20=Ish9NK%hQV-%fDsT0BViPbhA}V}#=&@qf(Z}}F%SzAVG_i_WS9a|As!MS5vD;B zBtr^Jhg6sWGhr6YhBTN1b73CLhXt?@7Qtdz0!v{TEQb}a5>~-#SOaTe9ju29un{)F zX4nGhuobq!cGv+KuoHH{ZrB5RVIS;=OgI1s;Sd~#BXAUs!ErbNC*c&FhBI&$&cS)O z02kpBT!t%f6|O-RT!$NQ6K=t6xC3|L9^8iq@DLusW5|Xl@D!fGb9ezS;T61wH}DqT z!F%`sAK?>xhA;3HzQK3+0YBjv{Dwd9*F@*fRD%haf)R2+PRIpjU=IJ5Et#yq8f?H8 zazh@lgS?Or@LKo-?-Jm=4fS%9`dP5lWfxgfW`ojPi2!kLT z2Ez~-3d3MHM8F7$gpn``M#C5w3*%rsM8O1zh8T#2i7*M`U@}aBsSpndkO~U^nc6y|54VLna)6gK!8A!x1G;5od2m+%T+!y9-D@8CUr zfRFGAKEoII3g6&6{D7bE3x2~N_-m^3XVhQ>w}XgZxkc3W7a2Kp`j$j^G4Ez!_YiC=>%%C=Ml{B$NU-C=F$xER+LxC=VV`0X(51 zR01!k3{{{iRD/.jpg`). Which survey that image comes from — POSS/DSS, SDSS, or future sources — is a **curation decision recorded once** in `object_images.source` and made **off-device**, never derived or branched on at runtime. This replaces the prior scheme, which fetched **both** POSS and SDSS into parallel `_POSS.jpg`/`_SDSS.jpg` files per object — of which the runtime only ever displayed POSS (`DM_SDSS` was defined but never assigned). + +The `source` record is both **retrospective** (where an image came from) and **prescriptive** (where to regenerate it from). A **discriminator** (human or AI — aspirational) compares candidate images across surveys and writes the winner; the dev-side **Generate** step reads `source` to (re)produce the single canonical image and publish it to the CDN. `NULL` means "not yet curated" — Generate falls back to a default policy (today: POSS). The on-device **Download** feature and the runtime **Display** path key purely on the sourceless `image_name`; they carry `source` but never act on it. + +## Considered Options +- **Keep both sources per object (status quo).** Rejected: ~doubles on-device storage and download time for an SDSS file no reachable UI path displayed. +- **Per-device source choice / toggle.** Rejected: premature with no working SDSS display mode, and it can't beat dev curation that picks the best image per object — while keeping the source dimension alive in storage, UI, and CDN. +- **Record the source choice in a dev-side manifest (not shipped).** Rejected: a second artifact to keep in sync with the CDN, and provenance/attribution isn't queryable on-device. The catalog DB already assigns the (sourceless) `image_name` in `resolve_object_images()`, so the same shipped table is the natural home for the choice. +- **Record nothing; let the CDN be the only record.** Rejected: non-reproducible (re-runs may flip the choice), non-auditable, and no path to survey attribution/credit. +- **Encode the source in the filename / CDN path (sourceless name but `M31_SDSS.jpg`).** Rejected: that's the scheme we're leaving; it forces resolution and display to know the source and reintroduces per-source duplication. + +## Consequences +- On-disk and CDN layout become `/.jpg`; `resolve_image_name` drops its `source` argument; `cat_images.get_display_image` drops the source concept; `DM_SDSS` is removed from `object_details`. +- `object_images` gains a nullable `source TEXT` column, populated by Generate/the discriminator and shipped in `pifinder_objects.db`. The column is provenance + regeneration directive only — no resolution or display code branches on it. +- A one-time, idempotent rename migration under `migration_source/` converts existing `_POSS`/`_SDSS` installs to the sourceless name (POSS wins when both exist; the redundant file is deleted). Reversing this decision would mean re-curating the CDN and reintroducing source suffixes across storage, the importer, and the UI — hence "hard to reverse." +- Generation (`gen_images.py`, off-device, multi-source, source-aware) and the new on-device download are cleanly separated: download copies one curated image by `image_name`; Generate is the only place a source is read or acted on. +- **Coordinated cutover:** the CDN is re-laid-out to sourceless paths (`//.jpg`) in the same release that ships the rename migration and the on-device download feature. The legacy `get_images.py` is replaced by a thin CLI over the shared core; a pre-release device's old CLI requests `_POSS`/`_SDSS` URLs and will 404 against the new CDN, so it must update first. Accepted because downloading is an explicit setup action, not a runtime dependency. +- The new `source` column ships with the (git-tracked) `pifinder_objects.db` via the normal software update — there is **no** on-device DB migration; the only on-device migration is the JPEG file rename. +- Companion glossary: [`docs/ax/catalog/CONTEXT.md`](../ax/catalog/CONTEXT.md) — "Object image", "Image source", "Discriminator", "Generate", "Download", "Display". diff --git a/docs/ax/catalog/CONTEXT.md b/docs/ax/catalog/CONTEXT.md index ec25401b4..f7cc61b78 100644 --- a/docs/ax/catalog/CONTEXT.md +++ b/docs/ax/catalog/CONTEXT.md @@ -203,6 +203,36 @@ _Avoid_: section header / section heading (that's the visual rendering of the la The description text an observing list carries for one of its targets — the observing-list counterpart of a **catalog description**. So an object's description can come from a catalog *or* from an observing list, and the **composed description** shows both. Session-only, held in `CompositeObject.list_descriptions` keyed by list name (re-loading a list replaces its own entry, never duplicates). Set only on **resolved** objects; a **coordinate object** has none (its list text becomes its own catalog-side description, since it has nothing else). _Avoid_: list note, note, comment, annotation, user description. +### Object images + +**Object image**: +The single survey JPEG shown behind a sky object in object details (the POSS/DSS plate, reddened and oriented to the eyepiece view). **Exactly one per object** — there is no per-source duplication on the device. Resolved from the object's **image_name** and rendered by `cat_images.get_display_image`, scaled/oriented by the active **Equipment** (see [ADR 0003](../../adr/0003-object-image-orientation.md)). +_Avoid_: POSS image / SDSS image (those name a **source**, not the stored image), thumbnail, finder image. + +**Image source** (survey): +The sky survey a curated **object image** is (re)generated from — POSS/DSS, SDSS, or future surveys. Recorded per object in `object_images.source` and authored by a **discriminator**. The record is both **retrospective** (where this image came from) and **prescriptive** (regenerate it from here): the dev-side **Generate** step reads it to (re)produce the one canonical image, and `NULL` means "not yet curated" — Generate falls back to a default policy. The source is deliberately **not** encoded in the on-disk name, the CDN path, or any resolution/display branch — the runtime carries the value but never acts on it (see [ADR 0018](../../adr/0018-one-object-image-per-object.md)). +_Avoid_: image type, image format; the historical `_POSS`/`_SDSS` filename suffix (a retired storage detail, not a live concept). + +**Discriminator**: +The (human or AI) curation role that compares an object's candidate images across **image sources** and chooses the best one, writing that choice to `object_images.source`. The authoritative producer of the **prescriptive** source record; the **Generate** step is its consumer. Aspirational: both the `source` column and the discriminator are introduced by [ADR 0018](../../adr/0018-one-object-image-per-object.md), not yet built. +_Avoid_: selector, picker, classifier (it discriminates *between candidate images*, not object types). + +**image_name**: +The **sourceless** filename stem for an object's image (`"M31"`), stored in the `object_images` table (one row per imaged sky object). Resolves on disk to `/.jpg`. Falls back to a whitespace-stripped common **Name** when the catalog-code+sequence stem has no file. +_Avoid_: image path, image id, image key. + +**Generate** (object images): +The **dev-side, off-device** step that fetches candidate cutouts from surveys, curates the single best per object, and publishes one sourceless **object image** to the CDN. The only place an **image source** is chosen. (`gen_images.py`.) +_Avoid_: fetch, build, import. + +**Download** (object images): +The **on-device** step that pulls already-curated object images for in-scope objects from the CDN to local disk. Carries no source dimension — it copies whatever the CDN holds for each **image_name**. Distinct from **Generate**: download never touches a survey. +_Avoid_: fetch, sync, get (the verb is "download"; reserve "generate" for the survey-side step). + +**Display** (object images): +The **runtime** step that loads the local object image and orients/crops/reddens it for the current eyepiece. Read-only; never fetches. (`cat_images.get_display_image`.) +_Avoid_: render (too generic), show. + ### UI helpers **CatalogDesignator**: diff --git a/docs/source/menu_map.rst b/docs/source/menu_map.rst index 50e705ac8..bd0c9eeed 100644 --- a/docs/source/menu_map.rst +++ b/docs/source/menu_map.rst @@ -291,6 +291,7 @@ information or perform actions. See :ref:`user_guide:tools`. Tools --> PnT["Place & Time"] Tools --> Console Tools --> SU["Software Upd"] + Tools --> DI["Download Images"] Tools --> TM["Test Mode"] Tools --> Exp["Experimental"] Tools --> Power @@ -340,6 +341,10 @@ Console Software Upd Download and install software updates over WiFi. See :ref:`user_guide:update software`. +Download Images + Fetch catalog object images on the device for a chosen scope — All Objects, + the Current Filter, or your Observing List. Requires WiFi Client mode. See + :ref:`software:catalog image download`. Test Mode A demo/debug mode that solves a saved image from disk. It blocks real use at night but lets you explore the PiFinder's features indoors. diff --git a/docs/source/software.rst b/docs/source/software.rst index 0408a5aef..cdc809f64 100644 --- a/docs/source/software.rst +++ b/docs/source/software.rst @@ -132,24 +132,27 @@ Booting takes up to two minutes, but you should see the startup screen before lo Catalog Image Download ^^^^^^^^^^^^^^^^^^^^^^ -The PiFinder can display catalog object images when they're present on your SD card. These images take about 5gb of space and can take several hours or more to download, but you can cancel and resume at any time. +The PiFinder can show a survey image behind each object in Object Details when the images are present on your SD card. They take a few GB of space and the full set can take a while to fetch, but you can cancel and pick up where you left off at any time. -The :ref:`software:prebuilt release image` already includes these images and is much quicker to download as a single file from your main computer. +The :ref:`software:prebuilt release image` already bundles every image, so flashing it is by far the fastest way to get them — one download on your main computer instead of thousands of small ones on the device. -To download the catalog images, put your PiFinder in WIFI client mode so it can reach the internet, then SSH into it using the password you set up initially. +To fetch them on the device, first put the PiFinder in WiFi Client mode so it can reach the internet (see :doc:`connectivity`). Then open Tools, select Download Images, and choose what to fetch: -Once connected, type: +* **All Objects** — every catalog image. +* **Current Filter** — only the objects your active filter currently shows. +* **Observing List** — the objects on your loaded observing list. -.. code-block:: +The PiFinder counts how many images are missing, estimates the download size, and checks that it can reach the image server, then offers Download or Cancel. (If WiFi isn't in Client mode the screen reads "WiFi must be client mode" — switch modes and come back.) - cd PiFinder/python - python -m PiFinder.get_images - -The PiFinder checks which images are missing and starts downloading. You can monitor progress on the status bar. +While a download runs, a status line just under the title bar shows its progress on every screen, so you can keep observing while it works. Pressing **LEFT** to go back leaves the download running in the background; the Cancel action on the Download Images screen stops it. Either way, images already downloaded are kept. +Downloading is idempotent: re-running a scope skips images you already have, so there's nothing special to resume — just trigger the same scope again. .. image:: ../../images/screenshots/Image_download_001.png :alt: Image Download - -There are 13,000+ images, so it takes a while, but you can do it across multiple sessions. The PiFinder uses whichever images you have on hand each time you observe. +.. note:: + There's also a headless fallback for advanced users. With the PiFinder in + Client mode, SSH in and run ``python -m PiFinder.get_images`` from + ``PiFinder/python`` to fetch every image, or add ``--catalog NGC`` to fetch a + single catalog. diff --git a/migration_source/v2.7.0.sh b/migration_source/v2.7.0.sh new file mode 100644 index 000000000..fa1a551e5 --- /dev/null +++ b/migration_source/v2.7.0.sh @@ -0,0 +1,6 @@ +# Convert legacy per-source object images to one sourceless image per object. +# Renames _POSS.jpg / _SDSS.jpg to .jpg (POSS wins when both +# exist; the redundant SDSS file is removed) so the device matches the +# sourceless CDN layout. Idempotent and version-gated by pifinder_post_update.sh. +# See docs/adr/0018-one-object-image-per-object.md. +python /home/pifinder/PiFinder/python/PiFinder/migrations/v2_7_0_sourceless_images.py /home/pifinder/PiFinder_data/catalog_images diff --git a/pifinder_post_update.sh b/pifinder_post_update.sh index 26372fd52..6e24c2cef 100644 --- a/pifinder_post_update.sh +++ b/pifinder_post_update.sh @@ -55,6 +55,14 @@ then touch /home/pifinder/PiFinder_data/migrations/v2.6.0 fi +# v2.7.0 +# Rename per-source object images (_POSS/_SDSS) to one sourceless image per object +if ! [ -f "/home/pifinder/PiFinder_data/migrations/v2.7.0" ] +then + source /home/pifinder/PiFinder/migration_source/v2.7.0.sh + touch /home/pifinder/PiFinder_data/migrations/v2.7.0 +fi + # DONE echo "Post Update Complete" diff --git a/python/PiFinder/audit_images.py b/python/PiFinder/audit_images.py deleted file mode 100644 index ef37fdb70..000000000 --- a/python/PiFinder/audit_images.py +++ /dev/null @@ -1,93 +0,0 @@ -#!/usr/bin/python -# -*- coding:utf-8 -*- -# mypy: ignore-errors -""" -This script runs to fetch -images from AWS -""" - -import requests -import sqlite3 -from tqdm import tqdm - -from PiFinder import cat_images - - -def get_catalog_objects(): - conn = sqlite3.connect( - "/Users/rich/Projects/Astronomy/PiFinder/astro_data/pifinder_objects.db" - ) - conn.row_factory = sqlite3.Row - cat_objects = conn.execute( - """ - SELECT * from objects - order by catalog desc ,sequence - """ - ).fetchall() - return cat_objects - - -def check_object_image(catalog_object): - """ - Check if image exists - or fetch it. - - Returns image path - """ - if catalog_object["catalog"] not in ["NGC", "IC"]: - # look for any NGC aka - conn = sqlite3.connect( - "/Users/rich/Projects/Astronomy/PiFinder/astro_data/pifinder_objects.db" - ) - conn.row_factory = sqlite3.Row - - aka_rec = conn.execute( - f""" - SELECT common_name from names - where catalog = "{catalog_object['catalog']}" - and sequence = "{catalog_object['sequence']}" - and common_name like "NGC%" - """ - ).fetchone() - if aka_rec: - try: - aka_sequence = int(aka_rec["common_name"][3:].strip()) - except ValueError: - aka_sequence = None - pass - - if aka_sequence: - catalog_object = {"catalog": "NGC", "sequence": aka_sequence} - - object_image_path = cat_images.resolve_image_name(catalog_object, "POSS") - # POSS - image_name = object_image_path.split("/")[-1] - seq_ones = image_name.split("_")[0][-1] - s3_url = ( - f"https://ddbeeedxfpnp0.cloudfront.net/catalog_images/{seq_ones}/{image_name}" - ) - r = requests.head(s3_url) - if r.status_code == 200: - return True - elif r.status_code == 403: - return False - else: - print(s3_url, r.status_code) - return False - - -def main(): - all_objects = get_catalog_objects() - print("Checking for missing images") - print(f"Checking {len(all_objects)} images....") - for catalog_object in tqdm(all_objects): - if not check_object_image(catalog_object): - print( - "Missing: " - + catalog_object["catalog"] - + str(catalog_object["sequence"]) - ) - - -if __name__ == "__main__": - main() diff --git a/python/PiFinder/cat_images.py b/python/PiFinder/cat_images.py index 1886c57e8..bc9ffb98c 100644 --- a/python/PiFinder/cat_images.py +++ b/python/PiFinder/cat_images.py @@ -11,10 +11,14 @@ from PIL import Image, ImageChops, ImageDraw from PiFinder import image_util from PiFinder import utils +from PiFinder.object_image_store import resolve_image_name import PiFinder.ui.ui_utils as ui_utils import logging -BASE_IMAGE_PATH = f"{utils.data_dir}/catalog_images" +# On-disk layout, image-dir creation and (sourceless) name resolution now live +# in the shared object-image core (object_image_store); see ADR 0018. This +# module keeps only the runtime **Display** logic and imports resolve_image_name +# for it below. CATALOG_PATH = f"{utils.astro_data_dir}/pifinder_objects.db" @@ -195,7 +199,7 @@ def get_display_image( roll: degrees """ - object_image_path = resolve_image_name(catalog_object, source="POSS") + object_image_path = resolve_image_name(catalog_object) logger.debug("object_image_path = %s", object_image_path) if not os.path.exists(object_image_path): return_image = Image.new("RGB", display_class.resolution) @@ -402,46 +406,3 @@ def get_display_image( ) return return_image - - -def resolve_image_name(catalog_object, source): - """ - returns the image path for this object - """ - - def create_image_path(image_name): - last_char = str(image_name)[-1] - image = f"{BASE_IMAGE_PATH}/{last_char}/{image_name}_{source}.jpg" - exists = os.path.exists(image) - return exists, image - - # Try primary name - image_name = f"{catalog_object.catalog_code}{catalog_object.sequence}" - ok, image = create_image_path(image_name) - - if ok: - catalog_object.image_name = image - return image - - # Try alternatives - for name in catalog_object.names: - alt_image_name = f"{''.join(name.split())}" - ok, image = create_image_path(alt_image_name) - if ok: - catalog_object.image_name = image - return image - - return "" - - -def create_catalog_image_dirs(): - """ - Checks for and creates catalog_image dirs - """ - if not os.path.exists(BASE_IMAGE_PATH): - os.makedirs(BASE_IMAGE_PATH) - - for i in range(0, 10): - _image_dir = f"{BASE_IMAGE_PATH}/{i}" - if not os.path.exists(_image_dir): - os.makedirs(_image_dir) diff --git a/python/PiFinder/catalog_imports/catalog_import_utils.py b/python/PiFinder/catalog_imports/catalog_import_utils.py index e828e37b2..ac9b2a558 100644 --- a/python/PiFinder/catalog_imports/catalog_import_utils.py +++ b/python/PiFinder/catalog_imports/catalog_import_utils.py @@ -440,7 +440,13 @@ def resolve_object_images(): else: unresolved_objects.append(object_id) - # Bulk insert image objects + # Bulk insert image objects. + # + # ``source`` is intentionally left NULL here: at import time every image is + # "uncurated". The survey provenance / regeneration directive is written + # off-device by Generate / the discriminator (ADR 0018); nothing on-device + # branches on it, so the importer only needs object_id + (sourceless) + # image_name. if image_objects_to_insert: # Use executemany for bulk insert into the correct table db_c.executemany( diff --git a/python/PiFinder/db/objects_db.py b/python/PiFinder/db/objects_db.py index 95eaa3c2b..1c63f4a07 100644 --- a/python/PiFinder/db/objects_db.py +++ b/python/PiFinder/db/objects_db.py @@ -97,12 +97,19 @@ def create_tables(self): ) # Create images_objects table + # + # ``source`` records the survey a curated object image was (re)generated + # from — a provenance + regeneration directive only (ADR 0018). It is + # written off-device by Generate / the discriminator and shipped in + # pifinder_objects.db; NULL means "uncurated" (Generate defaults to + # POSS). No on-device resolution or display code branches on it. self.cursor.execute( """ CREATE TABLE IF NOT EXISTS object_images ( id INTEGER PRIMARY KEY AUTOINCREMENT, object_id INTEGER, image_name TEXT, + source TEXT, FOREIGN KEY (object_id) REFERENCES objects(id) ); """ diff --git a/python/PiFinder/gen_images.py b/python/PiFinder/gen_images.py index 10e3ec9a3..dfbf0beeb 100644 --- a/python/PiFinder/gen_images.py +++ b/python/PiFinder/gen_images.py @@ -1,15 +1,24 @@ #!/usr/bin/python # -*- coding:utf-8 -*- """ -Fetch images from sky survey sources (NASA SkyView POSS, SDSS DR18) -and prepare them for PiFinder use. +**Generate** object images (ADR 0018) — the dev-side, off-device step. + +Fetches a cutout from a sky survey (NASA SkyView POSS, SDSS DR18) and writes one +**sourceless** image per object (``/.jpg``) ready to publish +to the CDN. Which survey to (re)generate from is read per object from +``object_images.source`` — the recorded curation directive — with ``NULL`` ("not +yet curated") falling back to POSS. This is the *only* place an image source is +read or acted on; resolution, display and the on-device download never branch on +it. + +Multi-source candidate staging and the discriminator (which would *write* +``source``) are out of v1 scope: this tool produces the single canonical image +for the recorded (or default) source. Usage: - python -m PiFinder.gen_images # Fetch missing images - python -m PiFinder.gen_images --force # Re-fetch ALL images - python -m PiFinder.gen_images --force --poss # Re-fetch POSS only - python -m PiFinder.gen_images --force --sdss # Re-fetch SDSS only - python -m PiFinder.gen_images --workers 20 # More concurrency + python -m PiFinder.gen_images # fetch missing images + python -m PiFinder.gen_images --force # re-fetch ALL images + python -m PiFinder.gen_images --workers 20 # more concurrency """ import argparse @@ -18,18 +27,17 @@ import time from concurrent.futures import ThreadPoolExecutor, as_completed from io import BytesIO -from typing import Dict, List, Tuple, cast +from typing import List, Tuple, cast import requests from PIL import Image, ImageOps from tqdm import tqdm +from PiFinder import object_image_store as store from PiFinder import utils -BASE_IMAGE_PATH = f"{utils.data_dir}/catalog_images" - -# Catalogs that are excluded from image fetching (no meaningful survey images) -EXCLUDED_CATALOGS = {"WDS"} +# Default survey for an uncurated (NULL/empty) ``object_images.source``. +DEFAULT_SOURCE = "POSS" SKYVIEW_URL = ( "https://skyview.gsfc.nasa.gov/current/cgi/runquery.pl" @@ -43,9 +51,14 @@ ) -def resolve_image_path(image_name: str, source: str) -> str: - last_char = str(image_name)[-1] - return f"{BASE_IMAGE_PATH}/{last_char}/{image_name}_{source}.jpg" +def survey_for(source) -> str: + """Survey to (re)generate this object's image from. + + Reads the recorded curation directive; ``NULL``/empty/unknown falls back to + the default (POSS). See ADR 0018. + """ + normalized = (source or "").strip().upper() + return normalized if normalized in ("POSS", "SDSS") else DEFAULT_SOURCE def check_sdss_image(image: Image.Image) -> bool: @@ -68,10 +81,9 @@ def check_sdss_image(image: Image.Image) -> bool: def fetch_poss( - session: requests.Session, ra: float, dec: float, image_name: str, low_cut: int = 10 + session: requests.Session, ra: float, dec: float, path: str, low_cut: int = 10 ) -> Tuple[bool, str]: - """Fetch POSS image from NASA SkyView.""" - path = resolve_image_path(image_name, "POSS") + """Fetch a POSS image from NASA SkyView and save it (sourceless) to ``path``.""" url = SKYVIEW_URL.format(ra=ra, dec=dec) try: resp = session.get(url, timeout=60) @@ -88,10 +100,9 @@ def fetch_poss( def fetch_sdss( - session: requests.Session, ra: float, dec: float, image_name: str + session: requests.Session, ra: float, dec: float, path: str ) -> Tuple[bool, str]: - """Fetch SDSS DR18 image.""" - path = resolve_image_path(image_name, "SDSS") + """Fetch an SDSS DR18 image and save it (sourceless) to ``path``.""" url = SDSS_URL.format(ra=ra, dec=dec) try: resp = session.get(url, timeout=60) @@ -114,119 +125,82 @@ def fetch_object( ra: float, dec: float, image_name: str, - do_poss: bool, - do_sdss: bool, + source, force: bool, -) -> Tuple[str, Dict[str, Tuple[bool, str]]]: - """Fetch survey images for one object.""" - results: Dict[str, Tuple[bool, str]] = {} - - if do_poss: - path = resolve_image_path(image_name, "POSS") - if force or not os.path.exists(path): - results["POSS"] = fetch_poss(session, ra, dec, image_name) - else: - results["POSS"] = (True, "exists") - - if do_sdss: - path = resolve_image_path(image_name, "SDSS") - if force or not os.path.exists(path): - results["SDSS"] = fetch_sdss(session, ra, dec, image_name) - else: - results["SDSS"] = (True, "exists") - - return image_name, results - - -def get_objects_to_fetch() -> List[Tuple[float, float, str]]: - """Get all non-WDS objects with their coordinates and image names.""" - db_path = utils.pifinder_db - conn = sqlite3.connect(str(db_path)) +) -> Tuple[str, str, Tuple[bool, str]]: + """Generate the one sourceless image for an object from its recorded source.""" + survey = survey_for(source) + path = store.local_image_path(image_name) + if not force and os.path.exists(path): + return image_name, survey, (True, "exists") + if survey == "SDSS": + result = fetch_sdss(session, ra, dec, path) + else: + result = fetch_poss(session, ra, dec, path) + return image_name, survey, result + + +def get_objects_to_fetch() -> List[Tuple[float, float, str, object]]: + """All CDN-eligible objects with coordinates, image name and recorded source.""" + conn = sqlite3.connect(str(utils.pifinder_db)) conn.row_factory = sqlite3.Row cursor = conn.cursor() - excluded_placeholders = ",".join("?" for _ in EXCLUDED_CATALOGS) + excluded = ",".join("?" for _ in store.EXCLUDED_CATALOGS) cursor.execute( f""" - SELECT DISTINCT o.ra, o.dec, oi.image_name + SELECT DISTINCT o.ra, o.dec, oi.image_name, oi.source FROM objects o JOIN object_images oi ON oi.object_id = o.id JOIN catalog_objects co ON co.object_id = o.id - WHERE co.catalog_code NOT IN ({excluded_placeholders}) + WHERE co.catalog_code NOT IN ({excluded}) AND oi.image_name != '' """, - list(EXCLUDED_CATALOGS), + list(store.EXCLUDED_CATALOGS), ) - rows = [(r["ra"], r["dec"], r["image_name"]) for r in cursor.fetchall()] + rows = [ + (r["ra"], r["dec"], r["image_name"], r["source"]) for r in cursor.fetchall() + ] conn.close() return rows -def create_catalog_image_dirs(): - if not os.path.exists(BASE_IMAGE_PATH): - os.makedirs(BASE_IMAGE_PATH) - for i in range(0, 10): - d = f"{BASE_IMAGE_PATH}/{i}" - if not os.path.exists(d): - os.makedirs(d) - - def main(): parser = argparse.ArgumentParser( - description="Fetch survey images for PiFinder objects" + description="Generate (survey-fetch) sourceless PiFinder object images" ) parser.add_argument( "--force", action="store_true", help="Re-fetch even if image exists" ) - parser.add_argument("--poss", action="store_true", help="Fetch POSS only") - parser.add_argument("--sdss", action="store_true", help="Fetch SDSS only") parser.add_argument( "--workers", type=int, default=10, help="Concurrent workers (default: 10)" ) args = parser.parse_args() - do_poss = True - do_sdss = True - if args.poss and not args.sdss: - do_sdss = False - elif args.sdss and not args.poss: - do_poss = False - - create_catalog_image_dirs() + store.create_catalog_image_dirs() print("Querying objects from database...") objects = get_objects_to_fetch() - print(f"Found {len(objects)} objects (excluding {', '.join(EXCLUDED_CATALOGS)})") + print( + f"Found {len(objects)} objects " + f"(excluding {', '.join(sorted(store.EXCLUDED_CATALOGS))})" + ) if args.force: to_fetch = objects print(f"Force mode: will re-fetch all {len(to_fetch)} objects") else: - to_fetch = [] - for ra, dec, name in objects: - poss_missing = do_poss and not os.path.exists( - resolve_image_path(name, "POSS") - ) - sdss_missing = do_sdss and not os.path.exists( - resolve_image_path(name, "SDSS") - ) - if poss_missing or sdss_missing: - to_fetch.append((ra, dec, name)) + to_fetch = [ + obj for obj in objects if not os.path.exists(store.local_image_path(obj[2])) + ] print(f"Missing images: {len(to_fetch)} of {len(objects)}") if not to_fetch: print("Nothing to fetch!") return - sources = [] - if do_poss: - sources.append("POSS") - if do_sdss: - sources.append("SDSS") - print(f"Fetching: {', '.join(sources)} with {args.workers} workers") - session = requests.Session() - session.headers.update({"User-Agent": "PiFinder-ImageGenerator/2.0"}) + session.headers.update({"User-Agent": "PiFinder-ImageGenerator/3.0"}) failed: List[Tuple[str, str]] = [] fetched = 0 @@ -236,30 +210,30 @@ def main(): with ThreadPoolExecutor(max_workers=args.workers) as executor: futures = { executor.submit( - fetch_object, session, ra, dec, name, do_poss, do_sdss, args.force + fetch_object, session, ra, dec, name, source, args.force ): name - for ra, dec, name in to_fetch + for ra, dec, name, source in to_fetch } for future in tqdm( - as_completed(futures), total=len(futures), desc="Downloading" + as_completed(futures), total=len(futures), desc="Generating" ): name = futures[future] try: - _, results = future.result() - for source, (ok, err) in results.items(): - if err == "exists": - skipped += 1 - elif ok: - fetched += 1 - else: - failed.append((f"{name}_{source}", err)) + _, survey, (ok, err) = future.result() + if err == "exists": + skipped += 1 + elif ok: + fetched += 1 + else: + failed.append((f"{name} ({survey})", err)) except Exception as e: failed.append((name, str(e))) elapsed = time.time() - t_start print( - f"\nDone in {elapsed:.0f}s: {fetched} fetched, {skipped} skipped, {len(failed)} failed" + f"\nDone in {elapsed:.0f}s: {fetched} fetched, {skipped} skipped, " + f"{len(failed)} failed" ) if failed: diff --git a/python/PiFinder/get_images.py b/python/PiFinder/get_images.py index 8bedfec7b..f5606ce09 100644 --- a/python/PiFinder/get_images.py +++ b/python/PiFinder/get_images.py @@ -1,171 +1,103 @@ #!/usr/bin/python # -*- coding:utf-8 -*- """ -This script runs to fetch -images from AWS +Thin CLI to download curated object images from the CDN (ADR 0018). + +For most users this is replaced by the on-device flow at +Tools ▸ Download Images. It is kept as a headless fallback for the two +scopes that are pure DB queries — **All** (every object minus the excluded +catalogs) and **single catalog** — which need no running app. The filter / +observing-list scopes need the live app and are device-only. + +All the real work (layout, worklist, sourceless download) lives in the shared +``object_image_store`` core; this module is just argument parsing and a progress +bar. + +Usage: + python -m PiFinder.get_images # all objects (minus WDS) + python -m PiFinder.get_images --catalog NGC # one catalog + python -m PiFinder.get_images --workers 8 # more concurrency + python -m PiFinder.get_images --overwrite # re-fetch existing files """ -import requests -import os -from tqdm import tqdm +import argparse from concurrent.futures import ThreadPoolExecutor, as_completed -from typing import List, Tuple -from PiFinder import cat_images +from tqdm import tqdm + +from PiFinder import object_image_store as store from PiFinder.db.objects_db import ObjectsDatabase -def check_missing_images() -> List[str]: - """ - Efficiently check which images need to be fetched by working directly - with image names from the database instead of creating CompositeObjects. +def main(): + parser = argparse.ArgumentParser( + description="Download PiFinder object images from the CDN" + ) + parser.add_argument( + "--catalog", + help="Limit to a single catalog code (e.g. NGC, M, IC). Default: all.", + ) + parser.add_argument( + "--workers", + type=int, + default=store.DEFAULT_DOWNLOAD_WORKERS, + help=f"Concurrent downloads (default: {store.DEFAULT_DOWNLOAD_WORKERS})", + ) + parser.add_argument( + "--overwrite", + action="store_true", + help="Re-download images that already exist", + ) + args = parser.parse_args() + + store.create_catalog_image_dirs() - Returns list of missing image names. - """ objects_db = ObjectsDatabase() _, cursor = objects_db.get_conn_cursor() - - # Get all image names directly from object_images table - cursor.execute( - "SELECT DISTINCT image_name FROM object_images WHERE image_name != ''" - ) - image_names = [row["image_name"] for row in cursor.fetchall()] - - missing_images = [] - for image_name in tqdm(image_names, desc="Checking existing images"): - # Check if POSS image exists (primary check) - poss_path = ( - f"{cat_images.BASE_IMAGE_PATH}/{image_name[-1]}/{image_name}_POSS.jpg" + if args.catalog: + names = store.worklist_for_scope( + store.SCOPE_CATALOG, cursor, catalog_code=args.catalog ) - if not os.path.exists(poss_path): - missing_images.append(image_name) - - return missing_images - - -def download_image_from_url( - session: requests.Session, url: str, file_path: str -) -> Tuple[bool, str]: - """ - Download a single image using provided session. - - Returns (success, error_message) - """ - try: - response = session.get(url, timeout=30) - if response.status_code == 200: - os.makedirs(os.path.dirname(file_path), exist_ok=True) - with open(file_path, "wb") as f: - f.write(response.content) - return True, "" - elif response.status_code == 403: - return False, "Not available (403)" - else: - return False, f"HTTP {response.status_code}" - except Exception as e: - return False, f"Error: {str(e)}" - - -def fetch_images_for_object( - session: requests.Session, image_name: str -) -> Tuple[str, bool, List[str]]: - """ - Fetch both POSS and SDSS images for a given image name. - - Returns (image_name, success, error_messages) - """ - errors = [] - seq_ones = image_name[-1] # Last character for directory - - # Download POSS image - poss_filename = f"{image_name}_POSS.jpg" - poss_path = f"{cat_images.BASE_IMAGE_PATH}/{seq_ones}/{poss_filename}" - poss_url = f"https://ddbeeedxfpnp0.cloudfront.net/catalog_images/{seq_ones}/{poss_filename}" - - poss_success, poss_error = download_image_from_url(session, poss_url, poss_path) - if not poss_success: - errors.append(f"POSS: {poss_error}") - - # Download SDSS image - sdss_filename = f"{image_name}_SDSS.jpg" - sdss_path = f"{cat_images.BASE_IMAGE_PATH}/{seq_ones}/{sdss_filename}" - sdss_url = f"https://ddbeeedxfpnp0.cloudfront.net/catalog_images/{seq_ones}/{sdss_filename}" - - sdss_success, sdss_error = download_image_from_url(session, sdss_url, sdss_path) - if not sdss_success: - errors.append(f"SDSS: {sdss_error}") - - # Consider successful if at least one image was downloaded - overall_success = poss_success or sdss_success - - return image_name, overall_success, errors - - -def download_images_concurrent(image_names: List[str], max_workers: int = 10) -> None: - """ - Download images concurrently using ThreadPoolExecutor. + else: + names = store.worklist_for_scope(store.SCOPE_ALL, cursor) - Args: - image_names: List of image names to download - max_workers: Maximum number of concurrent downloads - """ - if not image_names: + targets = names if args.overwrite else store.missing_image_names(names) + print(f"{len(names)} images in scope; {len(targets)} to download") + if not targets: + print("Nothing to download.") return - # Create a session for connection pooling - session = requests.Session() - session.headers.update({"User-Agent": "PiFinder-ImageDownloader/1.0"}) - - failed_downloads = [] - - with ThreadPoolExecutor(max_workers=max_workers) as executor: - # Submit all download tasks - future_to_image = { - executor.submit(fetch_images_for_object, session, image_name): image_name - for image_name in image_names - } - - # Process completed downloads with progress bar - for future in tqdm( - as_completed(future_to_image), - total=len(image_names), - desc="Downloading images", - ): - image_name = future_to_image[future] - try: - image_name, success, errors = future.result() - if not success and errors: - failed_downloads.append((image_name, errors)) - except Exception as exc: - failed_downloads.append((image_name, [f"Exception: {exc}"])) - - # Report failed downloads - if failed_downloads: - print(f"\nFailed to download {len(failed_downloads)} objects:") - for image_name, errors in failed_downloads[:10]: # Show first 10 failures - print(f" {image_name}: {', '.join(errors)}") - if len(failed_downloads) > 10: - print(f" ... and {len(failed_downloads) - 10} more") - - session.close() - - -def main(): - """ - Main function to check for and download missing catalog images. - """ - cat_images.create_catalog_image_dirs() - - print("Checking for missing images...") - missing_images = check_missing_images() - - if len(missing_images) > 0: - print(f"Found {len(missing_images)} objects with missing images") - print("Starting concurrent download...") - download_images_concurrent(missing_images, max_workers=10) - print("Download complete!") - else: - print("All images already downloaded!") + session = store.new_session() + counts = { + store.RESULT_DOWNLOADED: 0, + store.RESULT_SKIPPED: 0, + store.RESULT_MISSING: 0, + store.RESULT_ERROR: 0, + } + try: + with ThreadPoolExecutor(max_workers=args.workers) as executor: + futures = [ + executor.submit( + store.download_object_image, + session, + name, + overwrite=args.overwrite, + ) + for name in targets + ] + for future in tqdm( + as_completed(futures), total=len(futures), desc="Downloading" + ): + counts[future.result()] += 1 + finally: + session.close() + + print( + f"Done: {counts[store.RESULT_DOWNLOADED]} downloaded, " + f"{counts[store.RESULT_SKIPPED]} skipped, " + f"{counts[store.RESULT_MISSING]} missing (404), " + f"{counts[store.RESULT_ERROR]} errors" + ) if __name__ == "__main__": diff --git a/python/PiFinder/main.py b/python/PiFinder/main.py index 8754ff10d..e3649ddd4 100644 --- a/python/PiFinder/main.py +++ b/python/PiFinder/main.py @@ -47,6 +47,7 @@ from PiFinder.ui.console import UIConsole from PiFinder.ui.menu_manager import MenuManager +from PiFinder.object_image_download import ObjectImageDownloader from PiFinder.state import SharedStateObj, UIState @@ -635,6 +636,11 @@ def main( console.write(" Menus") console.update() + # App-owned object-image download worker. Lives for the life of the UI + # process so a download survives screen navigation; the download UI + # screens and the global title-bar status line share this one instance. + object_image_downloader = ObjectImageDownloader() + # Initialize menu manager menu_manager = MenuManager( display_device, @@ -643,6 +649,7 @@ def main( command_queues, cfg, catalogs, + object_image_downloader, ) # Initialize power manager diff --git a/python/PiFinder/migrations/v2_7_0_sourceless_images.py b/python/PiFinder/migrations/v2_7_0_sourceless_images.py new file mode 100644 index 000000000..d8a4722bc --- /dev/null +++ b/python/PiFinder/migrations/v2_7_0_sourceless_images.py @@ -0,0 +1,92 @@ +"""v2.7.0 migration: one sourceless object image per object. + +ADR 0018 moves object images from two source-suffixed files per object +(``_POSS.jpg`` / ``_SDSS.jpg``) to a single sourceless image +(``.jpg``), matching the re-laid-out CDN. This renames any legacy +suffixed files already on the device: + + * POSS wins when both a POSS and an SDSS file exist (the runtime only ever + displayed POSS); the redundant SDSS file is removed. + * If a sourceless ``.jpg`` already exists, the suffixed copies are just + removed as redundant. + +Walks ``//`` buckets under the catalog-images dir. Idempotent: +once converted there are no ``_POSS``/``_SDSS`` files left, so a re-run is a +no-op. Version-gated and run once by ``pifinder_post_update.sh``. + +Stdlib only: invoked by absolute file path from migration_source/v2.7.0.sh, so +it must not import PiFinder or rely on the working directory. +""" + +import os +import sys + +POSS_SUFFIX = "_POSS.jpg" +SDSS_SUFFIX = "_SDSS.jpg" + +DEFAULT_BASE_PATH = "/home/pifinder/PiFinder_data/catalog_images" + + +def _legacy_stems(file_names): + """Base stems in a bucket that have a legacy ``_POSS``/``_SDSS`` file.""" + stems = set() + for name in file_names: + if name.endswith(POSS_SUFFIX): + stems.add(name[: -len(POSS_SUFFIX)]) + elif name.endswith(SDSS_SUFFIX): + stems.add(name[: -len(SDSS_SUFFIX)]) + return stems + + +def migrate_images(base_path): + """Rename legacy suffixed images under ``base_path`` to sourceless names. + + Returns ``(renamed, removed)`` counts. No-op (returns ``(0, 0)``) when the + directory is missing; never creates it. + """ + renamed = 0 + removed = 0 + if not os.path.isdir(base_path): + return renamed, removed + + for bucket in os.listdir(base_path): + bucket_path = os.path.join(base_path, bucket) + if not os.path.isdir(bucket_path): + continue + + for stem in _legacy_stems(os.listdir(bucket_path)): + poss = os.path.join(bucket_path, stem + POSS_SUFFIX) + sdss = os.path.join(bucket_path, stem + SDSS_SUFFIX) + target = os.path.join(bucket_path, stem + ".jpg") + + if os.path.exists(poss): + chosen, other = poss, sdss + elif os.path.exists(sdss): + chosen, other = sdss, None + else: + continue + + if os.path.exists(target): + # Sourceless image already present; suffixed copies are redundant. + for path in (poss, sdss): + if os.path.exists(path): + os.remove(path) + removed += 1 + continue + + os.replace(chosen, target) + renamed += 1 + if other is not None and os.path.exists(other): + os.remove(other) + removed += 1 + + return renamed, removed + + +if __name__ == "__main__": + path = sys.argv[1] if len(sys.argv) > 1 else DEFAULT_BASE_PATH + renamed_count, removed_count = migrate_images(path) + print( + "v2.7.0 sourceless-images migration: " + f"renamed {renamed_count}, removed {removed_count} redundant" + ) diff --git a/python/PiFinder/object_image_download.py b/python/PiFinder/object_image_download.py new file mode 100644 index 000000000..f04a68b43 --- /dev/null +++ b/python/PiFinder/object_image_download.py @@ -0,0 +1,186 @@ +#!/usr/bin/python +# -*- coding:utf-8 -*- +""" +App-owned background **Download** worker for object images (ADR 0018). + +A single ``ObjectImageDownloader`` lives for the life of the main UI process +(wired in ``main.py``) and is shared by the download UI screens and the global +title-bar status line. It owns one background thread that fans a worklist of +sourceless ``image_name``s out over a small ``ThreadPoolExecutor`` (download is +I/O-bound, so the GIL is released on network/file I/O). + +Lifecycle (handoff "Lifecycle: manual re-trigger, idempotent skip"): + * One run at a time; ``start()`` is a no-op while a run is active. + * ``cancel()`` (or BACK) stops scheduling new work and lets in-flight fetches + finish; **already-downloaded files are kept** (each write is atomic). + * No auto-resume — re-running a scope just skips files already present. + +UI screens never touch the thread; they read an immutable :class:`DownloadProgress` +snapshot via :meth:`ObjectImageDownloader.progress` and call start / cancel. +""" + +import logging +import threading +from concurrent.futures import ThreadPoolExecutor, as_completed +from dataclasses import dataclass +from typing import List, Optional + +from PiFinder import object_image_store as store + +logger = logging.getLogger("Catalog.ImageDownload") + +# Run states. +IDLE = "idle" +RUNNING = "running" +DONE = "done" +CANCELLED = "cancelled" +ERROR = "error" + + +@dataclass(frozen=True) +class DownloadProgress: + """Immutable snapshot of a download run for the UI to visualize.""" + + state: str + total: int + downloaded: int + skipped: int + missing: int + errors: int + + @property + def completed(self) -> int: + """Images processed so far (downloaded + skipped + missing + errors).""" + return self.downloaded + self.skipped + self.missing + self.errors + + @property + def active(self) -> bool: + return self.state == RUNNING + + @property + def percent(self) -> int: + if self.total <= 0: + return 0 + return min(100, int(100 * self.completed / self.total)) + + +class ObjectImageDownloader: + """Background controller for on-device object-image downloads.""" + + def __init__(self, workers: int = store.DEFAULT_DOWNLOAD_WORKERS): + self._workers = workers + self._lock = threading.Lock() + self._cancel = threading.Event() + self._thread: Optional[threading.Thread] = None + + # Guarded by ``_lock``. + self._state = IDLE + self._total = 0 + self._downloaded = 0 + self._skipped = 0 + self._missing = 0 + self._errors = 0 + + # -- public API --------------------------------------------------------- # + def is_active(self) -> bool: + with self._lock: + return self._state == RUNNING + + def progress(self) -> DownloadProgress: + with self._lock: + return DownloadProgress( + state=self._state, + total=self._total, + downloaded=self._downloaded, + skipped=self._skipped, + missing=self._missing, + errors=self._errors, + ) + + def start(self, image_names: List[str]) -> bool: + """Begin downloading ``image_names`` in the background. + + Returns ``False`` (and does nothing) if a run is already active. + """ + with self._lock: + if self._state == RUNNING: + return False + self._cancel.clear() + self._state = RUNNING + self._total = len(image_names) + self._downloaded = 0 + self._skipped = 0 + self._missing = 0 + self._errors = 0 + thread = threading.Thread( + target=self._run, + args=(list(image_names),), + name="ObjectImageDownloader", + daemon=True, + ) + self._thread = thread + thread.start() + return True + + def cancel(self) -> None: + """Request cancellation; keeps files already written.""" + self._cancel.set() + + # -- worker ------------------------------------------------------------- # + def _tally(self, result: str) -> None: + with self._lock: + if result == store.RESULT_DOWNLOADED: + self._downloaded += 1 + elif result == store.RESULT_SKIPPED: + self._skipped += 1 + elif result == store.RESULT_MISSING: + self._missing += 1 + else: + self._errors += 1 + + def _fetch(self, session, image_name: str) -> str: + # Queued tasks short-circuit once cancellation is requested so the pool + # drains promptly without cancelling files already in flight. + if self._cancel.is_set(): + return store.RESULT_SKIPPED + return store.download_object_image(session, image_name) + + def _run(self, image_names: List[str]) -> None: + store.create_catalog_image_dirs() + session = store.new_session() + try: + with ThreadPoolExecutor(max_workers=self._workers) as executor: + futures = { + executor.submit(self._fetch, session, name): name + for name in image_names + } + for future in as_completed(futures): + try: + self._tally(future.result()) + except Exception as exc: # defensive: never kill the thread + logger.debug("download task failed: %s", exc) + self._tally(store.RESULT_ERROR) + if self._cancel.is_set(): + # Stop waiting on the rest; queued tasks short-circuit. + break + except Exception as exc: + logger.error("Object-image download run failed: %s", exc) + with self._lock: + self._state = ERROR + session.close() + return + finally: + session.close() + + with self._lock: + self._state = CANCELLED if self._cancel.is_set() else DONE + logger.info( + "Object-image download finished: %s (%d downloaded, %d skipped, " + "%d missing, %d errors of %d)", + self._state, + self._downloaded, + self._skipped, + self._missing, + self._errors, + self._total, + ) diff --git a/python/PiFinder/object_image_store.py b/python/PiFinder/object_image_store.py new file mode 100644 index 000000000..25f0e7574 --- /dev/null +++ b/python/PiFinder/object_image_store.py @@ -0,0 +1,296 @@ +#!/usr/bin/python +# -*- coding:utf-8 -*- +""" +Shared core for object images — ADR 0018: one sourceless image per object. + +Centralizes the on-disk layout, sourceless name resolution, per-scope worklist +building, and single-image CDN download shared by: + + * the runtime **Display** path (``cat_images.get_display_image``), + * the on-device **Download** feature (``object_image_download`` controller), + * the thin ``get_images`` CLI. + +An object image is stored sourceless as ``//.jpg`` +and served from the CDN at the matching path. Survey provenance ("source") is a +recorded-but-not-runtime-branched curation directive (``object_images.source``), +read only by the dev-side **Generate** step (``gen_images``); nothing in this +module branches on it. See ``docs/ax/catalog/CONTEXT.md`` → "Object images" and +``docs/adr/0018-one-object-image-per-object.md``. + +Deliberately lightweight (no PIL / no UI imports) so the CLI and the catalog +import tooling can use it without pulling in the display stack. +""" + +import logging +import os +from typing import List, Optional + +import requests + +from PiFinder import utils + +logger = logging.getLogger("Catalog.ImageStore") + +# Local, on-device root for downloaded object images. +BASE_IMAGE_PATH = f"{utils.data_dir}/catalog_images" + +# CloudFront host serving the curated, sourceless object images. Kept as a +# single module constant (not user config) — see the ADR consequences. +CDN_BASE_URL = "https://ddbeeedxfpnp0.cloudfront.net/catalog_images" + +# Catalogs with no meaningful survey image; excluded from the "All" scope and +# from the CDN. Matches ``gen_images`` (the publisher). +EXCLUDED_CATALOGS = {"WDS"} + +# Conservative, fixed concurrency for the on-device download worker. Downloads +# are I/O-bound (network + SD-card writes), so a small pool overlaps latency +# while keeping the Pi responsive. Not user-configurable in v1. +DEFAULT_DOWNLOAD_WORKERS = 4 + +# Per-image size estimate for the pre-flight ("~N MB to download"). The curated +# survey JPEGs vary; this is a deliberately rough average for a human-facing +# estimate only, never a hard limit. +ESTIMATED_IMAGE_BYTES = 120_000 + +REQUEST_TIMEOUT = 30 +_USER_AGENT = "PiFinder-ImageDownloader/2.0" + +# Download scopes (v1). All / single-catalog are pure DB queries and work from +# the thin CLI; filter / observing-list need the running app to supply the live +# objects (mapped via object_id to the canonical CDN ``image_name``). +SCOPE_ALL = "all" +SCOPE_CATALOG = "catalog" +SCOPE_FILTER = "filter" +SCOPE_LIST = "list" + +# Per-image download outcomes. +RESULT_DOWNLOADED = "downloaded" # fetched and written +RESULT_SKIPPED = "skipped" # already present locally (idempotent skip) +RESULT_MISSING = "missing" # 403/404 — not on the CDN (tolerated) +RESULT_ERROR = "error" # network/HTTP error + + +# --------------------------------------------------------------------------- # +# On-disk / CDN layout +# --------------------------------------------------------------------------- # +def image_bucket(image_name: str) -> str: + """Bucket sub-directory for an ``image_name`` (its last character).""" + return str(image_name)[-1] + + +def local_image_path(image_name: str) -> str: + """Absolute on-disk path for a sourceless object image.""" + return f"{BASE_IMAGE_PATH}/{image_bucket(image_name)}/{image_name}.jpg" + + +def cdn_image_url(image_name: str) -> str: + """CDN URL for a sourceless object image.""" + return f"{CDN_BASE_URL}/{image_bucket(image_name)}/{image_name}.jpg" + + +def create_catalog_image_dirs() -> None: + """Create the base image dir and its ten (0-9) bucket sub-directories.""" + if not os.path.exists(BASE_IMAGE_PATH): + os.makedirs(BASE_IMAGE_PATH) + for i in range(0, 10): + bucket = f"{BASE_IMAGE_PATH}/{i}" + if not os.path.exists(bucket): + os.makedirs(bucket) + + +def resolve_image_name(catalog_object) -> str: + """Local path of the displayable object image, or ``""`` if none on disk. + + Sourceless (ADR 0018): a single ``/.jpg`` per object. Tries + the viewed listing's ``catalog_code``+``sequence`` stem first, then falls + back to each whitespace-stripped common **Name** (this is what resolves an + object whose canonical image was published under another catalog's + designator — e.g. M 31 is stored as ``NGC224`` and matched via its + ``"NGC 224"`` name). + """ + + def _path_if_exists(stem: str): + path = local_image_path(stem) + return os.path.exists(path), path + + stem = f"{catalog_object.catalog_code}{catalog_object.sequence}" + ok, path = _path_if_exists(stem) + if ok: + return path + + for name in catalog_object.names: + ok, path = _path_if_exists("".join(name.split())) + if ok: + return path + + return "" + + +# --------------------------------------------------------------------------- # +# Worklists (which image_names a scope needs) +# --------------------------------------------------------------------------- # +def all_image_names(cursor) -> List[str]: + """Every CDN-eligible ``image_name`` (all catalogs minus the excluded set).""" + placeholders = ",".join("?" for _ in EXCLUDED_CATALOGS) + rows = cursor.execute( + f""" + SELECT DISTINCT oi.image_name + FROM object_images oi + JOIN catalog_objects co ON co.object_id = oi.object_id + WHERE co.catalog_code NOT IN ({placeholders}) + AND oi.image_name != '' + """, + list(EXCLUDED_CATALOGS), + ).fetchall() + return [row["image_name"] for row in rows] + + +def catalog_image_names(cursor, catalog_code: str) -> List[str]: + """Every ``image_name`` reachable through a single catalog's listings.""" + rows = cursor.execute( + """ + SELECT DISTINCT oi.image_name + FROM object_images oi + JOIN catalog_objects co ON co.object_id = oi.object_id + WHERE co.catalog_code = ? + AND oi.image_name != '' + """, + (catalog_code,), + ).fetchall() + return [row["image_name"] for row in rows] + + +def image_names_for_object_ids(cursor, object_ids) -> List[str]: + """Canonical ``image_name``s for a set of sky-object ids. + + Used by the filter / observing-list scopes: the live ``CompositeObject``s + carry an ``object_id`` but their ``image_name`` attribute is unset until + display, so the canonical CDN stem is looked up here (one row per imaged + sky object), independent of which catalog listing the user is viewing. + """ + ids = sorted({int(oid) for oid in object_ids}) + if not ids: + return [] + # Chunk the IN list so a large filter / observing list stays under SQLite's + # bound-variable limit (999 on older builds). + chunk_size = 900 + names: List[str] = [] + seen = set() + for start in range(0, len(ids), chunk_size): + chunk = ids[start : start + chunk_size] + placeholders = ",".join("?" for _ in chunk) + rows = cursor.execute( + f""" + SELECT DISTINCT image_name + FROM object_images + WHERE object_id IN ({placeholders}) + AND image_name != '' + """, + chunk, + ).fetchall() + for row in rows: + name = row["image_name"] + if name not in seen: + seen.add(name) + names.append(name) + return names + + +def worklist_for_scope( + scope: str, + cursor, + *, + catalog_code: Optional[str] = None, + objects: Optional[list] = None, +) -> List[str]: + """Return the distinct ``image_name``s a download *scope* covers. + + * ``SCOPE_ALL`` / ``SCOPE_CATALOG`` — pure DB queries (also usable headless). + * ``SCOPE_FILTER`` / ``SCOPE_LIST`` — derived from caller-supplied live + ``objects`` (their ``object_id``s), mapped to canonical ``image_name``s. + """ + if scope == SCOPE_ALL: + return all_image_names(cursor) + if scope == SCOPE_CATALOG: + if not catalog_code: + raise ValueError("SCOPE_CATALOG requires catalog_code") + return catalog_image_names(cursor, catalog_code) + if scope in (SCOPE_FILTER, SCOPE_LIST): + object_ids = [getattr(obj, "object_id") for obj in (objects or [])] + return image_names_for_object_ids(cursor, object_ids) + raise ValueError(f"unknown download scope: {scope!r}") + + +def missing_image_names(image_names) -> List[str]: + """Subset of ``image_names`` not yet present on local disk (download targets).""" + return [name for name in image_names if not os.path.exists(local_image_path(name))] + + +def estimated_download_bytes(missing_count: int) -> int: + """Rough total size estimate for the pre-flight, in bytes.""" + return missing_count * ESTIMATED_IMAGE_BYTES + + +# --------------------------------------------------------------------------- # +# Download (one curated, sourceless image) +# --------------------------------------------------------------------------- # +def new_session() -> requests.Session: + """A pooled ``requests`` session with the PiFinder user-agent.""" + session = requests.Session() + session.headers.update({"User-Agent": _USER_AGENT}) + return session + + +def cdn_reachable(session: Optional[requests.Session] = None, timeout: int = 8) -> bool: + """Quick reachability probe of the CDN host for the pre-flight. + + A reachable CloudFront may answer the bare host path with 403/404; anything + short of a transport failure (or 5xx) counts as reachable. + """ + own = session is None + session = session or new_session() + try: + resp = session.head(f"{CDN_BASE_URL}/", timeout=timeout) + return resp.status_code < 500 + except requests.RequestException: + return False + finally: + if own: + session.close() + + +def download_object_image( + session: requests.Session, + image_name: str, + *, + overwrite: bool = False, + timeout: int = REQUEST_TIMEOUT, +) -> str: + """Download one sourceless object image to local disk. + + Idempotent: an already-present file is left untouched (``RESULT_SKIPPED``) + unless ``overwrite`` is set. 404/403 means the CDN has no image for this + name and is tolerated (``RESULT_MISSING``). Writes atomically via a + ``.part`` temp file so a cancel / power-loss never leaves a truncated JPEG. + """ + dest = local_image_path(image_name) + if not overwrite and os.path.exists(dest): + return RESULT_SKIPPED + + try: + resp = session.get(cdn_image_url(image_name), timeout=timeout) + except requests.RequestException as exc: + logger.debug("download error for %s: %s", image_name, exc) + return RESULT_ERROR + + if resp.status_code == 200: + os.makedirs(os.path.dirname(dest), exist_ok=True) + tmp = f"{dest}.part" + with open(tmp, "wb") as out: + out.write(resp.content) + os.replace(tmp, dest) + return RESULT_DOWNLOADED + if resp.status_code in (403, 404): + return RESULT_MISSING + logger.debug("download for %s returned HTTP %s", image_name, resp.status_code) + return RESULT_ERROR diff --git a/python/PiFinder/ui/base.py b/python/PiFinder/ui/base.py index 812c2bfab..9cb14fd4f 100644 --- a/python/PiFinder/ui/base.py +++ b/python/PiFinder/ui/base.py @@ -128,6 +128,9 @@ class UIModule: _unmoved = False # has the telescope moved since the last cam solve? _display_mode_list: Union[list[None], list[str]] = [None] # List of display modes marking_menu: Union[None, MarkingMenu] = None + # The dedicated download progress screen visualizes the run itself, so it + # suppresses the global title-bar status line to avoid drawing it twice. + _suppress_download_status = False def __init__( self, @@ -141,6 +144,7 @@ def __init__( add_to_stack=None, remove_from_stack=None, jump_to_label=None, + object_image_downloader=None, ): assert shared_state is not None self.title = self.__title__ @@ -155,6 +159,10 @@ def __init__( self.add_to_stack = add_to_stack self.remove_from_stack = remove_from_stack self.jump_to_label = jump_to_label + # App-owned background object-image downloader (None in tests / headless + # construction). Drawn as a global status line under the title bar while + # a download is active; the download screens drive it. + self.object_image_downloader = object_image_downloader # mode stuff self._display_mode_cycle = cycle(self._display_mode_list) @@ -376,6 +384,39 @@ def _draw_battery_icon(self, fg) -> bool: return True + def _draw_download_status_line(self) -> None: + """Global status line under the title bar while a download is active. + + Shows only the object-image download counts / percent (never IP/SSID). + Drawn as an overlay strip — it does not reflow each screen's content — + and is hidden when idle. Screens that visualize the run themselves set + ``_suppress_download_status``. + """ + downloader = self.object_image_downloader + if downloader is None or self._suppress_download_status: + return + progress = downloader.progress() + if not progress.active: + return + + tb_height = self.display_class.titlebar_height + line_height = self.fonts.base.height + 2 + self.draw.rectangle( + [0, tb_height, self.display_class.resX, tb_height + line_height], + fill=self.colors.get(32), + ) + text = _("Img {done}/{total} {pct}%").format( + done=progress.completed, + total=progress.total, + pct=progress.percent, + ) + self.draw.text( + (3, tb_height + 1), + text, + font=self.fonts.base.font, + fill=self.colors.get(192), + ) + def _draw_titlebar_rotating_info(self, x, y, fg): """Draw rotating constellation/SQM in title bar (dark text on gray bg).""" self._rotating_display.draw( @@ -491,6 +532,10 @@ def screen_update(self, title_bar=True, button_hints=True) -> None: fill=fg, ) + # Global download status line (overlay just under the title bar, + # hidden when no download is active). + self._draw_download_status_line() + # FPS self.frame_count += 1 if int(time.time()) - self.last_fps_sample_time > 0: diff --git a/python/PiFinder/ui/image_download.py b/python/PiFinder/ui/image_download.py new file mode 100644 index 000000000..4d74e1adb --- /dev/null +++ b/python/PiFinder/ui/image_download.py @@ -0,0 +1,412 @@ +#!/usr/bin/python +# -*- coding:utf-8 -*- +""" +On-device **Download** UI for object images (ADR 0018). + +Replaces the old SSH / ``python -m PiFinder.get_images`` step with an on-device +flow under Tools ▸ Download Images. A scope-picker submenu (menu_structure) +launches this screen with a ``scope`` in its item definition; the screen then: + + 1. gates on WiFi **Client** mode (reusing the software-update pattern), + 2. runs a **pre-flight** — missing count, rough size estimate, and a quick CDN + reachability check — in a background thread so the UI stays smooth, + 3. starts the app-owned :class:`ObjectImageDownloader` and visualizes its + progress, and + 4. shows a final summary. + +The worker is app-owned and survives navigation: pressing BACK while a download +runs leaves it going (the global title-bar status line keeps it visible); a +separate Cancel action stops it, keeping files already downloaded. +""" + +import logging +import threading +import time +from typing import Any, List, Optional, TYPE_CHECKING + +from PiFinder import utils +from PiFinder import object_image_store as store +from PiFinder.db.objects_db import ObjectsDatabase +from PiFinder.ui.base import UIModule + +if TYPE_CHECKING: + + def _(a) -> Any: + return a + + +logger = logging.getLogger("UIImageDownload") + +# Screen phases. +PHASE_PREFLIGHT = "preflight" +PHASE_RUNNING = "running" +PHASE_DONE = "done" + + +def _format_size(num_bytes: int) -> str: + """Human-facing size estimate (``~12 MB`` / ``~640 KB``).""" + mb = num_bytes / 1_000_000 + if mb >= 1: + return _("~{n} MB").format(n=round(mb)) + return _("~{n} KB").format(n=max(1, round(num_bytes / 1000))) + + +class UIImageDownload(UIModule): + """Download object images from the CDN to local disk, for a chosen scope.""" + + __title__ = "IMAGES" + # This screen visualizes the run itself, so suppress the global title-bar + # download status line while it is on top of the stack. + _suppress_download_status = True + + def __init__(self, *args, **kwargs) -> None: + super().__init__(*args, **kwargs) + self._scope = self.item_definition.get("scope", store.SCOPE_ALL) + self._catalog_code = self.item_definition.get("catalog_code") + + self.wifi_txt = f"{utils.pifinder_dir}/wifi_status.txt" + self._wifi_mode = "" + + self._phase = PHASE_PREFLIGHT + self._preflight_started = False + + # Pre-flight results (written by the background thread, read in update()). + self._lock = threading.Lock() + self._preflight_ready = False + self._preflight_error: Optional[str] = None + self._total = 0 + self._missing_names: List[str] = [] + self._reachable = False + + self._objects: Optional[list] = None # live objects for filter/list scopes + self._option_select = "Download" # "Download" | "Cancel" + self._elipsis_count = 0 + self._final = None # captured DownloadProgress when a run finishes + + # ------------------------------------------------------------------ # + # Lifecycle + # ------------------------------------------------------------------ # + def active(self): + super().active() + self._wifi_mode = self._read_wifi_mode() + + # If a download is already running (started from another scope), just + # show its progress rather than a new pre-flight. + if self.object_image_downloader and self.object_image_downloader.is_active(): + self._phase = PHASE_RUNNING + return + + if not self._preflight_started and self._wifi_mode == "Client": + self._preflight_started = True + # Gather live objects on the UI thread for the filter / list scopes + # (the background thread only touches the DB, disk and network). + if self._scope == store.SCOPE_FILTER: + self._objects = list( + self.catalogs.get_objects(only_selected=True, filtered=True) + ) + elif self._scope == store.SCOPE_LIST: + self._objects = list(self.ui_state.observing_list()) + threading.Thread(target=self._compute_preflight, daemon=True).start() + + def _read_wifi_mode(self) -> str: + try: + with open(self.wifi_txt, "r") as wfs: + return wfs.read().strip() + except OSError: + return "" + + def _worklist(self, cursor) -> List[str]: + if self._scope in (store.SCOPE_ALL, store.SCOPE_CATALOG): + return store.worklist_for_scope( + self._scope, cursor, catalog_code=self._catalog_code + ) + return store.worklist_for_scope(self._scope, cursor, objects=self._objects) + + def _compute_preflight(self): + """Background: build the worklist, count missing, probe the CDN.""" + try: + db = ObjectsDatabase() + _, cursor = db.get_conn_cursor() + names = self._worklist(cursor) + missing = store.missing_image_names(names) + reachable = store.cdn_reachable() + with self._lock: + self._total = len(names) + self._missing_names = missing + self._reachable = reachable + self._preflight_ready = True + except Exception as exc: + logger.error("Image-download pre-flight failed: %s", exc) + with self._lock: + self._preflight_error = str(exc) + self._preflight_ready = True + + # ------------------------------------------------------------------ # + # Drawing + # ------------------------------------------------------------------ # + def update(self, force=False): + time.sleep(1 / 30) + self.clear_screen() + + if self._phase == PHASE_RUNNING: + # Leave the running view once the worker finishes. + if ( + self.object_image_downloader + and not self.object_image_downloader.is_active() + ): + self._final = self.object_image_downloader.progress() + self._phase = PHASE_DONE + else: + self._draw_running() + return self.screen_update() + + if self._phase == PHASE_DONE: + self._draw_done() + return self.screen_update() + + self._draw_preflight() + return self.screen_update() + + def _scope_label(self) -> str: + if self._scope == store.SCOPE_ALL: + return _("All objects") + if self._scope == store.SCOPE_CATALOG: + return _("Catalog {code}").format(code=self._catalog_code or "?") + if self._scope == store.SCOPE_FILTER: + return _("Current filter") + if self._scope == store.SCOPE_LIST: + return _("Observing list") + return str(self._scope) + + def _draw_preflight(self): + y = self.display_class.titlebar_height + 2 + + self.draw.text( + (0, y), + self._scope_label(), + font=self.fonts.bold.font, + fill=self.colors.get(192), + ) + y += self.fonts.bold.height + 4 + + if self._wifi_mode != "Client": + self.draw.text( + (0, y), + _("WiFi must be"), + font=self.fonts.large.font, + fill=self.colors.get(255), + ) + self.draw.text( + (0, y + self.fonts.large.height), + _("client mode"), + font=self.fonts.large.font, + fill=self.colors.get(255), + ) + return + + with self._lock: + ready = self._preflight_ready + error = self._preflight_error + total = self._total + missing = len(self._missing_names) + reachable = self._reachable + + if not ready: + self._elipsis_count = (self._elipsis_count + 1) % 40 + self.draw.text( + (0, y), + _("Checking{e}").format(e="." * int(self._elipsis_count / 10)), + font=self.fonts.large.font, + fill=self.colors.get(255), + ) + return + + if error: + self.draw.text( + (0, y), + _("Error"), + font=self.fonts.large.font, + fill=self.colors.get(255), + ) + return + + # Counts + self.draw.text( + (0, y), + _("{n} of {t} missing").format(n=missing, t=total), + font=self.fonts.base.font, + fill=self.colors.get(192), + ) + y += self.fonts.base.height + 2 + self.draw.text( + (0, y), + _("Size {s}").format( + s=_format_size(store.estimated_download_bytes(missing)) + ), + font=self.fonts.base.font, + fill=self.colors.get(128), + ) + y += self.fonts.base.height + 2 + self.draw.text( + (0, y), + _("CDN ok") if reachable else _("CDN unreachable"), + font=self.fonts.base.font, + fill=self.colors.get(128 if reachable else 200), + ) + + # Action prompts, anchored from the bottom. + if missing == 0: + self.draw.text( + (0, self.display_class.resY - self.fonts.large.height - 2), + _("All present"), + font=self.fonts.large.font, + fill=self.colors.get(255), + ) + return + if not reachable: + return + + self._draw_action_options() + + def _draw_action_options(self): + pitch = self.fonts.large.height + top = self.display_class.resY - 2 * pitch - 4 + bottom = top + pitch + self.draw.text( + (10, top), + _("Download"), + font=self.fonts.large.font, + fill=self.colors.get(255), + ) + self.draw.text( + (10, bottom), + _("Cancel"), + font=self.fonts.large.font, + fill=self.colors.get(255), + ) + ind = top if self._option_select == "Download" else bottom + self.draw.text( + (0, ind), + self._RIGHT_ARROW, + font=self.fonts.large.font, + fill=self.colors.get(255), + ) + + def _draw_progress_bar(self, y, percent): + bar_x = round(self.display_class.resX * 4 / 128) + bar_w = self.display_class.resX - 2 * bar_x + bar_h = round(self.display_class.resY * 12 / 128) + self.draw.rectangle( + [bar_x, y, bar_x + bar_w, y + bar_h], + outline=self.colors.get(64), + ) + fill_w = int(bar_w * percent / 100) + if fill_w > 0: + self.draw.rectangle( + [bar_x + 1, y + 1, bar_x + fill_w, y + bar_h - 1], + fill=self.colors.get(255), + ) + pct_text = f"{percent}%" + pct_bbox = self.fonts.base.font.getbbox(pct_text) + pct_x = bar_x + (bar_w - (pct_bbox[2] - pct_bbox[0])) // 2 + pct_y = y + (bar_h - (pct_bbox[3] - pct_bbox[1])) // 2 - pct_bbox[1] + self.draw.text( + (pct_x, pct_y), + pct_text, + font=self.fonts.base.font, + fill=self.colors.get(0) if percent > 45 else self.colors.get(192), + ) + return y + bar_h + 4 + + def _draw_running(self): + progress = self.object_image_downloader.progress() + y = self.display_class.titlebar_height + 2 + self.draw.text( + (0, y), + _("Downloading"), + font=self.fonts.bold.font, + fill=self.colors.get(255), + ) + y += self.fonts.bold.height + 6 + y = self._draw_progress_bar(y, progress.percent) + y += 4 + self.draw.text( + (0, y), + _("{done}/{total} images").format( + done=progress.completed, total=progress.total + ), + font=self.fonts.base.font, + fill=self.colors.get(192), + ) + # Bottom hint: BACK keeps it running in the background, cancel stops it. + self.draw.text( + (0, self.display_class.resY - self.fonts.base.height - 1), + _("←keep bg {a}cancel").format(a=self._RIGHT_ARROW), + font=self.fonts.base.font, + fill=self.colors.get(96), + ) + + def _draw_done(self): + progress = self._final + y = self.display_class.titlebar_height + 2 + title = ( + _("Cancelled") if progress and progress.state == "cancelled" else _("Done") + ) + self.draw.text( + (0, y), + title, + font=self.fonts.bold.font, + fill=self.colors.get(255), + ) + y += self.fonts.bold.height + 6 + if progress: + for label, value in ( + (_("Downloaded"), progress.downloaded), + (_("Skipped"), progress.skipped), + (_("Missing"), progress.missing), + (_("Errors"), progress.errors), + ): + self.draw.text( + (0, y), + f"{label}: {value}", + font=self.fonts.base.font, + fill=self.colors.get(160), + ) + y += self.fonts.base.height + 2 + + # ------------------------------------------------------------------ # + # Keys + # ------------------------------------------------------------------ # + def _start_download(self): + with self._lock: + names = list(self._missing_names) + if not names or self.object_image_downloader is None: + return + if self.object_image_downloader.start(names): + self._phase = PHASE_RUNNING + + def key_up(self): + if self._phase == PHASE_PREFLIGHT: + self._option_select = ( + "Cancel" if self._option_select == "Download" else "Download" + ) + + def key_down(self): + self.key_up() + + def key_right(self): + if self._phase == PHASE_PREFLIGHT: + if self._option_select == "Cancel": + self.remove_from_stack() + else: + self._start_download() + elif self._phase == PHASE_RUNNING: + # Explicit cancel (keeps already-downloaded files). + if self.object_image_downloader: + self.object_image_downloader.cancel() + + def key_left(self) -> bool: + # BACK leaves the screen. A running download is app-owned and keeps + # going in the background (shown by the global title-bar status line); + # it is not cancelled by simply navigating away. + return True diff --git a/python/PiFinder/ui/menu_manager.py b/python/PiFinder/ui/menu_manager.py index f6b6c993d..11fdc9562 100644 --- a/python/PiFinder/ui/menu_manager.py +++ b/python/PiFinder/ui/menu_manager.py @@ -113,6 +113,7 @@ def __init__( command_queues, config_object, catalogs, + object_image_downloader=None, ): self.display_class = display_class self.shared_state = shared_state @@ -121,6 +122,10 @@ def __init__( self.command_queues = command_queues self.config_object = config_object self.catalogs = catalogs + # App-owned background object-image downloader (may be None in tests / + # headless construction); passed to every UI module so screens can drive + # it and the shared title bar can show a progress line. + self.object_image_downloader = object_image_downloader # stack switch anim stuff self._stack_anim_counter: float = 0 @@ -185,6 +190,7 @@ def preload_modules(self) -> None: add_to_stack=self.add_to_stack, remove_from_stack=self.remove_from_stack, jump_to_label=self.jump_to_label, + object_image_downloader=self.object_image_downloader, ) def add_to_stack(self, item: dict) -> None: @@ -210,6 +216,7 @@ def add_to_stack(self, item: dict) -> None: add_to_stack=self.add_to_stack, remove_from_stack=self.remove_from_stack, jump_to_label=self.jump_to_label, + object_image_downloader=self.object_image_downloader, ) ) if item.get("stateful", False): diff --git a/python/PiFinder/ui/menu_structure.py b/python/PiFinder/ui/menu_structure.py index 5a8b97f66..589c6e76c 100644 --- a/python/PiFinder/ui/menu_structure.py +++ b/python/PiFinder/ui/menu_structure.py @@ -5,6 +5,7 @@ from PiFinder.ui.status import UIStatus from PiFinder.ui.console import UIConsole from PiFinder.ui.software import UISoftware +from PiFinder.ui.image_download import UIImageDownload from PiFinder.ui.gpsstatus import UIGPSStatus from PiFinder.ui.chart import UIChart from PiFinder.ui.align import UIAlign @@ -1233,6 +1234,28 @@ def _(key: str) -> Any: }, {"name": _("Console"), "class": UIConsole}, {"name": _("Software Upd"), "class": UISoftware}, + { + "name": _("Download Images"), + "class": UITextMenu, + "select": "single", + "items": [ + { + "name": _("All Objects"), + "class": UIImageDownload, + "scope": "all", + }, + { + "name": _("Current Filter"), + "class": UIImageDownload, + "scope": "filter", + }, + { + "name": _("Observing List"), + "class": UIImageDownload, + "scope": "list", + }, + ], + }, {"name": _("Test Mode"), "callback": callbacks.activate_debug}, { "name": _("Experimental"), diff --git a/python/PiFinder/ui/object_details.py b/python/PiFinder/ui/object_details.py index c142519ab..639d33924 100644 --- a/python/PiFinder/ui/object_details.py +++ b/python/PiFinder/ui/object_details.py @@ -49,11 +49,13 @@ def _catalog_db() -> ObjectsDatabase: return _objects_db -# Constants for display modes +# Constants for display modes. +# There is exactly one object image per object (ADR 0018); DM_POSS is the image +# view. The old DM_SDSS mode was never assigned (no SDSS display path ever +# existed) and was removed with the move to one sourceless image per object. DM_DESC = 0 # Display mode for description DM_LOCATE = 1 # Display mode for LOCATE -DM_POSS = 2 # Display mode for POSS -DM_SDSS = 3 # Display mode for SDSS +DM_POSS = 2 # Display mode for the object image DM_CONTRAST = 4 # Display mode for Contrast Reserve explanation @@ -379,7 +381,7 @@ def update_object_info(self): self.config_object.equipment.calc_tfov(), roll, self.display_class, - burn_in=self.object_display_mode in [DM_POSS, DM_SDSS], + burn_in=self.object_display_mode == DM_POSS, magnification=magnification, show_nsew=self.config_object.get_option("image_nsew", True), show_bbox=self.config_object.get_option("image_bbox", True), @@ -497,7 +499,7 @@ def update(self, force=True): self.clear_screen() # paste image - if self.object_display_mode in [DM_POSS, DM_SDSS]: + if self.object_display_mode == DM_POSS: self.screen.paste(self.object_image) if self.object_display_mode == DM_DESC or self.object_display_mode == DM_LOCATE: @@ -746,7 +748,6 @@ def serialize_ui_state(self) -> dict: DM_DESC: "description", DM_LOCATE: "locate", DM_POSS: "poss_image", - DM_SDSS: "sdss_image", } # Serialize the object information safely diff --git a/python/tests/test_migration_sourceless_images.py b/python/tests/test_migration_sourceless_images.py new file mode 100644 index 000000000..e045f738d --- /dev/null +++ b/python/tests/test_migration_sourceless_images.py @@ -0,0 +1,76 @@ +"""Unit tests for the v2.7.0 sourceless-images rename migration (ADR 0018).""" + +import importlib.util +import pathlib + +import pytest + +# The migration is stdlib-only and invoked by absolute path on-device (it must +# not import PiFinder), so load it directly from its file. +_MIGRATION_PATH = ( + pathlib.Path(__file__).resolve().parents[1] + / "PiFinder" + / "migrations" + / "v2_7_0_sourceless_images.py" +) +_spec = importlib.util.spec_from_file_location( + "v2_7_0_sourceless_images", _MIGRATION_PATH +) +migration = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(migration) + + +def _bucket(base, image_name): + d = base / image_name[-1] + d.mkdir(parents=True, exist_ok=True) + return d + + +@pytest.mark.unit +class TestMigration: + def test_poss_wins_when_both_present(self, tmp_path): + b = _bucket(tmp_path, "NGC224") + (b / "NGC224_POSS.jpg").write_text("poss") + (b / "NGC224_SDSS.jpg").write_text("sdss") + + renamed, removed = migration.migrate_images(str(tmp_path)) + + assert (renamed, removed) == (1, 1) + assert (b / "NGC224.jpg").read_text() == "poss" + assert not (b / "NGC224_POSS.jpg").exists() + assert not (b / "NGC224_SDSS.jpg").exists() + + def test_sdss_only_is_renamed(self, tmp_path): + b = _bucket(tmp_path, "NGC3031") + (b / "NGC3031_SDSS.jpg").write_text("sdss") + + migration.migrate_images(str(tmp_path)) + + assert (b / "NGC3031.jpg").read_text() == "sdss" + assert not (b / "NGC3031_SDSS.jpg").exists() + + def test_existing_sourceless_target_kept_legacy_removed(self, tmp_path): + b = _bucket(tmp_path, "IC10") + (b / "IC10.jpg").write_text("existing") + (b / "IC10_POSS.jpg").write_text("legacy") + + renamed, removed = migration.migrate_images(str(tmp_path)) + + assert (renamed, removed) == (0, 1) + assert (b / "IC10.jpg").read_text() == "existing" + assert not (b / "IC10_POSS.jpg").exists() + + def test_idempotent(self, tmp_path): + b = _bucket(tmp_path, "NGC224") + (b / "NGC224_POSS.jpg").write_text("poss") + (b / "NGC224_SDSS.jpg").write_text("sdss") + + first = migration.migrate_images(str(tmp_path)) + second = migration.migrate_images(str(tmp_path)) + + assert first == (1, 1) + assert second == (0, 0) # nothing left to do + assert (b / "NGC224.jpg").read_text() == "poss" + + def test_missing_dir_is_noop(self, tmp_path): + assert migration.migrate_images(str(tmp_path / "does_not_exist")) == (0, 0) diff --git a/python/tests/test_object_image_download.py b/python/tests/test_object_image_download.py new file mode 100644 index 000000000..d0d0c4f9a --- /dev/null +++ b/python/tests/test_object_image_download.py @@ -0,0 +1,120 @@ +"""Unit tests for the app-owned object-image download controller (ADR 0018).""" + +import threading +import time +import types + +import pytest + +from PiFinder import object_image_download as dl +from PiFinder import object_image_store as store + + +def _wait_idle(downloader, timeout=5.0): + deadline = time.time() + timeout + while downloader.is_active() and time.time() < deadline: + time.sleep(0.01) + assert not downloader.is_active(), "download did not finish in time" + + +@pytest.fixture(autouse=True) +def _no_real_io(monkeypatch): + # Never touch the disk or network from the controller in these tests. + monkeypatch.setattr(store, "create_catalog_image_dirs", lambda: None) + monkeypatch.setattr( + store, "new_session", lambda: types.SimpleNamespace(close=lambda: None) + ) + + +@pytest.mark.unit +class TestController: + def test_runs_to_completion(self, monkeypatch): + monkeypatch.setattr( + store, + "download_object_image", + lambda session, name, **kw: store.RESULT_DOWNLOADED, + ) + downloader = dl.ObjectImageDownloader(workers=2) + assert downloader.start(["A", "B", "C"]) + _wait_idle(downloader) + + progress = downloader.progress() + assert progress.state == dl.DONE + assert progress.total == 3 + assert progress.downloaded == 3 + assert progress.completed == 3 + assert progress.percent == 100 + assert not progress.active + + def test_tallies_mixed_results(self, monkeypatch): + outcomes = { + "A": store.RESULT_DOWNLOADED, + "B": store.RESULT_SKIPPED, + "C": store.RESULT_MISSING, + "D": store.RESULT_ERROR, + } + monkeypatch.setattr( + store, + "download_object_image", + lambda session, name, **kw: outcomes[name], + ) + downloader = dl.ObjectImageDownloader(workers=4) + downloader.start(list(outcomes)) + _wait_idle(downloader) + + progress = downloader.progress() + assert ( + progress.downloaded, + progress.skipped, + progress.missing, + progress.errors, + ) == (1, 1, 1, 1) + assert progress.state == dl.DONE + + def test_second_start_while_active_is_rejected(self, monkeypatch): + release = threading.Event() + monkeypatch.setattr( + store, + "download_object_image", + lambda session, name, **kw: (release.wait(2), store.RESULT_DOWNLOADED)[1], + ) + downloader = dl.ObjectImageDownloader(workers=1) + assert downloader.start(["A"]) + assert downloader.start(["B"]) is False # already running + release.set() + _wait_idle(downloader) + + def test_cancel_marks_cancelled_and_keeps_progress(self, monkeypatch): + release = threading.Event() + + def slow(session, name, **kw): + release.wait(2) + return store.RESULT_DOWNLOADED + + monkeypatch.setattr(store, "download_object_image", slow) + downloader = dl.ObjectImageDownloader(workers=1) + downloader.start(["A", "B", "C", "D"]) + downloader.cancel() + release.set() + _wait_idle(downloader) + assert downloader.progress().state == dl.CANCELLED + + +@pytest.mark.unit +class TestProgressSnapshot: + def test_percent_and_completed(self): + p = dl.DownloadProgress( + state=dl.RUNNING, + total=200, + downloaded=10, + skipped=5, + missing=4, + errors=1, + ) + assert p.completed == 20 + assert p.percent == 10 + assert p.active + + def test_zero_total_percent(self): + p = dl.DownloadProgress(dl.IDLE, 0, 0, 0, 0, 0) + assert p.percent == 0 diff --git a/python/tests/test_object_image_store.py b/python/tests/test_object_image_store.py new file mode 100644 index 000000000..2fc5970a8 --- /dev/null +++ b/python/tests/test_object_image_store.py @@ -0,0 +1,210 @@ +"""Unit tests for the shared object-image core (ADR 0018).""" + +import sqlite3 +import types + +import pytest + +from PiFinder import object_image_store as store + + +def _obj(catalog_code, sequence, names=()): + """Minimal stand-in for a CompositeObject for resolution tests.""" + return types.SimpleNamespace( + catalog_code=catalog_code, sequence=sequence, names=list(names) + ) + + +@pytest.mark.unit +class TestLayout: + def test_bucket_is_last_char(self): + assert store.image_bucket("NGC224") == "4" + assert store.image_bucket("M13") == "3" + + def test_local_path_is_sourceless(self): + assert store.local_image_path("NGC224").endswith("/4/NGC224.jpg") + + def test_cdn_url_is_sourceless(self): + assert store.cdn_image_url("NGC224") == f"{store.CDN_BASE_URL}/4/NGC224.jpg" + + +@pytest.mark.unit +class TestResolveImageName: + def test_primary_stem(self, tmp_path, monkeypatch): + monkeypatch.setattr(store, "BASE_IMAGE_PATH", str(tmp_path)) + (tmp_path / "3").mkdir() + (tmp_path / "3" / "M13.jpg").write_bytes(b"x") + assert store.resolve_image_name(_obj("M", 13)) == str( + tmp_path / "3" / "M13.jpg" + ) + + def test_common_name_fallback(self, tmp_path, monkeypatch): + # M 31's canonical image is published as NGC224; the "NGC 224" common + # name (whitespace-stripped) is what resolves it on disk. + monkeypatch.setattr(store, "BASE_IMAGE_PATH", str(tmp_path)) + (tmp_path / "4").mkdir() + (tmp_path / "4" / "NGC224.jpg").write_bytes(b"x") + obj = _obj("M", 31, names=["NGC 224", "Andromeda Galaxy"]) + assert store.resolve_image_name(obj) == str(tmp_path / "4" / "NGC224.jpg") + + def test_missing_returns_empty(self, tmp_path, monkeypatch): + monkeypatch.setattr(store, "BASE_IMAGE_PATH", str(tmp_path)) + assert store.resolve_image_name(_obj("M", 99, names=["Nope"])) == "" + + def test_does_not_mutate_object(self, tmp_path, monkeypatch): + # image_name is the DB stem; resolution must not overwrite it with a path. + monkeypatch.setattr(store, "BASE_IMAGE_PATH", str(tmp_path)) + (tmp_path / "3").mkdir() + (tmp_path / "3" / "M13.jpg").write_bytes(b"x") + obj = _obj("M", 13) + obj.image_name = "M13" + store.resolve_image_name(obj) + assert obj.image_name == "M13" + + +@pytest.fixture +def cursor(): + conn = sqlite3.connect(":memory:") + conn.row_factory = sqlite3.Row + cur = conn.cursor() + cur.execute( + "CREATE TABLE object_images " + "(id INTEGER PRIMARY KEY, object_id INTEGER, image_name TEXT, source TEXT)" + ) + cur.execute( + "CREATE TABLE catalog_objects " + "(id INTEGER PRIMARY KEY, object_id INTEGER, catalog_code TEXT, sequence INTEGER)" + ) + rows_images = [ + (224, "NGC224"), # M 31 ≡ NGC 224 + (7000, "NGC7000"), # NGC only + (500, "WDS500"), # WDS only -> excluded from "All" + (9, ""), # empty image_name -> ignored everywhere + ] + cur.executemany( + "INSERT INTO object_images (object_id, image_name) VALUES (?, ?)", rows_images + ) + rows_listings = [ + (224, "NGC", 224), + (224, "M", 31), + (7000, "NGC", 7000), + (500, "WDS", 500), + (9, "M", 9), + ] + cur.executemany( + "INSERT INTO catalog_objects (object_id, catalog_code, sequence) " + "VALUES (?, ?, ?)", + rows_listings, + ) + conn.commit() + yield cur + conn.close() + + +@pytest.mark.unit +class TestWorklist: + def test_all_excludes_wds_and_empty(self, cursor): + assert set(store.worklist_for_scope(store.SCOPE_ALL, cursor)) == { + "NGC224", + "NGC7000", + } + + def test_single_catalog_ngc(self, cursor): + assert set( + store.worklist_for_scope(store.SCOPE_CATALOG, cursor, catalog_code="NGC") + ) == {"NGC224", "NGC7000"} + + def test_single_catalog_m_resolves_canonical_stem(self, cursor): + # Viewing the M listing still maps to the canonical NGC224 stem; the + # empty-image M 9 listing is excluded. + assert set( + store.worklist_for_scope(store.SCOPE_CATALOG, cursor, catalog_code="M") + ) == {"NGC224"} + + def test_filter_scope_maps_object_ids(self, cursor): + objects = [types.SimpleNamespace(object_id=224)] + assert store.worklist_for_scope( + store.SCOPE_FILTER, cursor, objects=objects + ) == ["NGC224"] + + def test_list_scope_dedups_and_skips_imageless(self, cursor): + objects = [ + types.SimpleNamespace(object_id=224), + types.SimpleNamespace(object_id=7000), + types.SimpleNamespace(object_id=9), # empty image_name + ] + assert set( + store.worklist_for_scope(store.SCOPE_LIST, cursor, objects=objects) + ) == {"NGC224", "NGC7000"} + + def test_empty_objects_returns_empty(self, cursor): + assert store.worklist_for_scope(store.SCOPE_FILTER, cursor, objects=[]) == [] + + def test_catalog_scope_requires_code(self, cursor): + with pytest.raises(ValueError): + store.worklist_for_scope(store.SCOPE_CATALOG, cursor) + + def test_unknown_scope_raises(self, cursor): + with pytest.raises(ValueError): + store.worklist_for_scope("bogus", cursor) + + +@pytest.mark.unit +class TestMissing: + def test_missing_image_names(self, tmp_path, monkeypatch): + monkeypatch.setattr(store, "BASE_IMAGE_PATH", str(tmp_path)) + (tmp_path / "4").mkdir() + (tmp_path / "4" / "NGC224.jpg").write_bytes(b"x") + assert store.missing_image_names(["NGC224", "NGC7000"]) == ["NGC7000"] + + +class _FakeResponse: + def __init__(self, status_code, content=b"jpegdata"): + self.status_code = status_code + self.content = content + + +class _FakeSession: + def __init__(self, status_code): + self._status_code = status_code + self.urls = [] + + def get(self, url, timeout=None): + self.urls.append(url) + return _FakeResponse(self._status_code) + + +@pytest.mark.unit +class TestDownloadObjectImage: + def test_downloads_to_sourceless_path(self, tmp_path, monkeypatch): + monkeypatch.setattr(store, "BASE_IMAGE_PATH", str(tmp_path)) + result = store.download_object_image(_FakeSession(200), "NGC224") + assert result == store.RESULT_DOWNLOADED + assert (tmp_path / "4" / "NGC224.jpg").read_bytes() == b"jpegdata" + + def test_skips_existing(self, tmp_path, monkeypatch): + monkeypatch.setattr(store, "BASE_IMAGE_PATH", str(tmp_path)) + (tmp_path / "4").mkdir() + (tmp_path / "4" / "NGC224.jpg").write_bytes(b"old") + session = _FakeSession(200) + assert store.download_object_image(session, "NGC224") == store.RESULT_SKIPPED + assert session.urls == [] # never hit the network + assert (tmp_path / "4" / "NGC224.jpg").read_bytes() == b"old" + + def test_overwrite(self, tmp_path, monkeypatch): + monkeypatch.setattr(store, "BASE_IMAGE_PATH", str(tmp_path)) + (tmp_path / "4").mkdir() + (tmp_path / "4" / "NGC224.jpg").write_bytes(b"old") + result = store.download_object_image( + _FakeSession(200), "NGC224", overwrite=True + ) + assert result == store.RESULT_DOWNLOADED + assert (tmp_path / "4" / "NGC224.jpg").read_bytes() == b"jpegdata" + + def test_404_is_missing(self, tmp_path, monkeypatch): + monkeypatch.setattr(store, "BASE_IMAGE_PATH", str(tmp_path)) + assert ( + store.download_object_image(_FakeSession(404), "NGC7000") + == store.RESULT_MISSING + ) + assert not (tmp_path / "0" / "NGC7000.jpg").exists() diff --git a/python/tests/test_ui_modules.py b/python/tests/test_ui_modules.py index e94a5400f..366090bc3 100644 --- a/python/tests/test_ui_modules.py +++ b/python/tests/test_ui_modules.py @@ -345,12 +345,19 @@ def _fast_sleep(): @pytest.fixture(scope="session", autouse=True) def _no_network(): - """Stub UISoftware's live GitHub version check - the one hard network call).""" + """Stub the UI's live network calls during the sweep. + + Two callers reach the network: UISoftware's GitHub version check, and the + object-image download pre-flight's CDN reachability probe (UIImageDownload + spawns it from active() when WiFi is in client mode). + """ fake = mock.Mock() fake.text = "1.0.0" fake.status_code = 200 - with mock.patch("PiFinder.ui.software.requests.get", return_value=fake): + with ( + mock.patch("PiFinder.ui.software.requests.get", return_value=fake), + mock.patch("PiFinder.object_image_store.cdn_reachable", return_value=True), + ): yield