Adds XTC support#31
Conversation
📝 WalkthroughWalkthroughThe PR extends CrossPoint Reader plugin to support XTC files in addition to EPUB. The device driver version was bumped, supported format declarations were expanded, file listing logic was updated to include XTC files, and documentation was clarified to reflect dual format support. ChangesXTC Format Support for CrossPoint Reader
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crosspoint_reader/driver.py (2)
410-416:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTemp file always uses '.epub' suffix regardless of actual file type.
The
_download_tempmethod hardcodessuffix='.epub'even when downloading XTC files. This could cause issues with metadata extraction or other tools that rely on file extensions.🔧 Proposed fix to use correct suffix based on file extension
- def _download_temp(self, path): + def _download_temp(self, path, suffix=None): from calibre.ptempfile import PersistentTemporaryFile - tf = PersistentTemporaryFile(suffix='.epub') + if suffix is None: + suffix = os.path.splitext(path)[1] or '.epub' + tf = PersistentTemporaryFile(suffix=suffix) self.get_file(path, tf) tf.flush() tf.seek(0)Then update the call site on line 188 if needed, or rely on the automatic suffix detection from the path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crosspoint_reader/driver.py` around lines 410 - 416, The _download_temp function always creates a PersistentTemporaryFile with suffix='.epub', which is incorrect for XTC or other file types; change it to derive the suffix from the incoming path (e.g., take the extension from path and pass suffix=f'.{ext}' or omit suffix to let the OS choose) when creating PersistentTemporaryFile in _download_temp, ensure you still call self.get_file(path, tf), flush and seek as before, and if any callers (e.g., the call site referenced in the review) assume '.epub' adjust them to handle the actual suffix or rely on automatic detection.
184-194:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftFix hardcoded
stream_type='epub'when reading metadata for non-EPUB inputs (e.g., XTC)
- The code always calls Calibre
get_metadata(..., stream_type='epub', ...), so when processing XTC it will force the EPUB metadata reader path.- Calibre’s
get_metadatadispatcher does not provide any support/remapping forstream_type='xtc'(noxtcreader path in the dispatcher), so changing the call tostream_type='xtc'won’t solve it.- Adjust the logic to use a Calibre-supported
stream_typefor XTC, or avoid forcingstream_typeand ensure Calibre can autodetect from the temp file type (so metadata isn’t dropped into the exception handler whenfetch_metadatais enabled).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crosspoint_reader/driver.py` around lines 184 - 194, The metadata reader is forcing stream_type='epub' in the fetch_metadata block which breaks non‑EPUB inputs (e.g., XTC); update the logic in the fetch_metadata branch that calls get_metadata to either omit the stream_type argument so Calibre will autodetect from the temp file created by _download_temp(lpath), or choose a Calibre‑supported stream_type based on the input extension (inspect lpath suffix/mime) before calling get_metadata; keep the existing quick_metadata context and preserve the current exception logging via self._log if autodetect still fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crosspoint_reader/driver.py`:
- Around line 410-416: The _download_temp function always creates a
PersistentTemporaryFile with suffix='.epub', which is incorrect for XTC or other
file types; change it to derive the suffix from the incoming path (e.g., take
the extension from path and pass suffix=f'.{ext}' or omit suffix to let the OS
choose) when creating PersistentTemporaryFile in _download_temp, ensure you
still call self.get_file(path, tf), flush and seek as before, and if any callers
(e.g., the call site referenced in the review) assume '.epub' adjust them to
handle the actual suffix or rely on automatic detection.
- Around line 184-194: The metadata reader is forcing stream_type='epub' in the
fetch_metadata block which breaks non‑EPUB inputs (e.g., XTC); update the logic
in the fetch_metadata branch that calls get_metadata to either omit the
stream_type argument so Calibre will autodetect from the temp file created by
_download_temp(lpath), or choose a Calibre‑supported stream_type based on the
input extension (inspect lpath suffix/mime) before calling get_metadata; keep
the existing quick_metadata context and preserve the current exception logging
via self._log if autodetect still fails.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8329df17-8409-4325-ae4e-2329ade195e5
⛔ Files ignored due to path filters (2)
crosspoint_reader-v0.1.1.zipis excluded by!**/*.zipcrosspoint_reader-v0.1.5.zipis excluded by!**/*.zip
📒 Files selected for processing (3)
README.mdcrosspoint_reader/README.mdcrosspoint_reader/driver.py
📜 Review details
🧰 Additional context used
🪛 Ruff (0.15.14)
crosspoint_reader/driver.py
[warning] 27-27: Mutable default value for class attribute
(RUF012)
[warning] 28-28: Mutable default value for class attribute
(RUF012)
[warning] 29-29: Mutable default value for class attribute
(RUF012)
[warning] 31-31: Mutable default value for class attribute
(RUF012)
[warning] 32-32: Mutable default value for class attribute
(RUF012)
🔇 Additional comments (4)
crosspoint_reader/driver.py (2)
24-24: LGTM!Also applies to: 31-32
154-154: ⚡ Quick winVerify/correct the server’s XTC flag casing to ensure XTC files are included.
crosspoint_reader/driver.pychecksentry.get('isXTC')(line 171) while there are no otherisXtc/isxtcreferences in the repo. If the server/API returns a differently cased field (e.g.,isXtc), XTC files won’t be listed—confirm the exact/api/filesresponse field name and update the condition to match (or accept both casings).README.md (1)
9-9: LGTM!crosspoint_reader/README.md (1)
4-4: LGTM!
1 participant
Crosspoint can display EPUB and XTC files but previously only EPUB files could be sent from Calibre to Crosspoint devices using this plugin. This adds XTC support for sending to Crosspoint devices.