Skip to content

The Filewatcher expect a list of str#2

Merged
JarbasAl merged 2 commits into
OpenVoiceOS:devfrom
devbar:dev
Dec 17, 2025
Merged

The Filewatcher expect a list of str#2
JarbasAl merged 2 commits into
OpenVoiceOS:devfrom
devbar:dev

Conversation

@devbar

@devbar devbar commented Sep 16, 2025

Copy link
Copy Markdown
Contributor
  1. The Filewatcher expects in current version of ovos_utils a list of paths. Sending string leads to the interpretation of the string as a list of chars. Watching for file will always ends with "file not found".
  2. The read_chunk method should read the whole queue of bytes to send the expected result to the backend

@coderabbitai

coderabbitai Bot commented Sep 16, 2025

Copy link
Copy Markdown

Walkthrough

Updated start() in ovos_microphone_plugin_files/__init__.py to instantiate FileWatcher with a list containing self.files_folder instead of a single string path. Callback usage and other logic remain unchanged.

Changes

Cohort / File(s) Summary of Changes
FileWatcher init usage
ovos_microphone_plugin_files/__init__.py
Changed FileWatcher(self.files_folder, callback=self.on_new_file) to FileWatcher([self.files_folder], callback=self.on_new_file) to pass a list of folders instead of a single path. Public API usage adjusted accordingly; no other logic modified.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I twitch my ears at paths I see,
One string split to a list—yippee! 🐇
A watcher hops through folders now,
Same callback, same solemn vow.
Files appear, I thump with cheer,
Small change made crystal clear.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title accurately reflects the primary change of updating the FileWatcher to require a list of string paths rather than a single string, making it clear and specific to the main change made in the code. Despite a minor grammatical error in verb agreement, the title remains directly relevant and informative for reviewers scanning the history.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
ovos_microphone_plugin_files/__init__.py (1)

64-65: Pass list to FileWatcher — correct. Also ensure the directory exists before starting the watcher.

Change fixes the ovos_utils API expectation. To avoid init failures when the default folder doesn’t exist, create it first.

Apply this diff:

 def start(self):
     assert self._watcher is None, "Already started"
-    self._watcher = FileWatcher([self.files_folder],
-                                callback=self.on_new_file)
+    os.makedirs(self.files_folder, exist_ok=True)
+    self._watcher = FileWatcher([self.files_folder],
+                                callback=self.on_new_file)
     self._is_running = True

Follow‑ups:

  • Verify minimum ovos_utils version that requires List[str] and pin it in packaging to prevent breakage on older installs.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 93bed3b and f9fe2fb.

📒 Files selected for processing (1)
  • ovos_microphone_plugin_files/__init__.py (1 hunks)

@devbar

devbar commented Nov 25, 2025

Copy link
Copy Markdown
Contributor Author

Hi guys, I think this change still needs approval

@JarbasAl JarbasAl self-assigned this Nov 25, 2025
@JarbasAl JarbasAl merged commit fabcdd9 into OpenVoiceOS:dev Dec 17, 2025
1 check passed
@JarbasAl

Copy link
Copy Markdown
Member

@devbar sorry it took me so long! thanks for the PR

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.

2 participants