Skip to content

Fix #166#177

Merged
Eeems merged 1 commit intomainfrom
Eeems-patch-2
Apr 16, 2026
Merged

Fix #166#177
Eeems merged 1 commit intomainfrom
Eeems-patch-2

Conversation

@Eeems
Copy link
Copy Markdown
Collaborator

@Eeems Eeems commented Apr 16, 2026

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced device status detection to gracefully handle unavailable configuration files without errors.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 16, 2026

Walkthrough

The DeviceManager.get_device_status() method now explicitly initializes version_id and beta_contents to empty strings upfront. An SFTP-specific helper function using ftp.stat checks file existence before remote reads. Conditional file access for /etc/version and /home/root/.config/remarkable/xochitl.conf replaces previous fallback assignments.

Changes

Cohort / File(s) Summary
Device Status Reading
codexctl/device.py
Added upfront variable initialization and introduced local exists() helper for SFTP to conditionally guard remote file reads; removed previous fallback assignments for missing files.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Fix #166' is minimal and refers to an issue, but provides no specific information about what was actually fixed or changed in the code. Consider updating the title to describe the specific fix, such as 'Add conditional file existence checks to prevent SFTP failures' or similar wording that reflects the actual code changes.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch Eeems-patch-2

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Eeems

This comment was marked as resolved.

@coderabbitai

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
codexctl/device.py (1)

514-519: Consider extracting the SFTP exists helper to avoid a third copy.

This is the third near-identical file_exists/exists helper in the module (see _read_version_from_path at line 309 and _get_paper_pro_partition_info at line 459). Extracting a small private method like _sftp_exists(ftp, path) would eliminate the duplication and keep behavior (exception types caught) consistent across call sites.

♻️ Sketch
+    `@staticmethod`
+    def _sftp_exists(ftp, path: str) -> bool:
+        try:
+            ftp.stat(path)
+            return True
+        except (FileNotFoundError, IOError):
+            return False

Then at line 514:

-            def exists(path:str) -> bool:
-                try:
-                    _ = ftp.stat(path)
-                    return True
-                except (FileNotFoundError, IOError):
-                    return False
+            exists = lambda p: self._sftp_exists(ftp, p)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@codexctl/device.py` around lines 514 - 519, Extract the duplicated SFTP
existence check into a single private helper (e.g. _sftp_exists(ftp, path)) and
replace the three local helpers (the exists defined near line 514, the
file_exists/_read_version_from_path helper around line 309, and the helper used
in _get_paper_pro_partition_info around line 459) to call this new function; the
helper should accept the FTP/SFTP client and path, try ftp.stat(path) and return
True, and catch the same exceptions currently used (FileNotFoundError, IOError)
to return False so behavior remains consistent across call sites.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@codexctl/device.py`:
- Around line 514-519: Extract the duplicated SFTP existence check into a single
private helper (e.g. _sftp_exists(ftp, path)) and replace the three local
helpers (the exists defined near line 514, the
file_exists/_read_version_from_path helper around line 309, and the helper used
in _get_paper_pro_partition_info around line 459) to call this new function; the
helper should accept the FTP/SFTP client and path, try ftp.stat(path) and return
True, and catch the same exceptions currently used (FileNotFoundError, IOError)
to return False so behavior remains consistent across call sites.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 685303cd-7a8c-44f3-9bdb-0c1de5fc5850

📥 Commits

Reviewing files that changed from the base of the PR and between 693f0b6 and 1f0c0c2.

📒 Files selected for processing (1)
  • codexctl/device.py

@Eeems Eeems merged commit b7c4bde into main Apr 16, 2026
14 checks passed
@Eeems Eeems deleted the Eeems-patch-2 branch April 16, 2026 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant