Skip to content

luci-app-watchcat: add support for 'enabled' option - #8930

Open
Ser9ei wants to merge 1 commit into
openwrt:masterfrom
Ser9ei:watchcat
Open

luci-app-watchcat: add support for 'enabled' option#8930
Ser9ei wants to merge 1 commit into
openwrt:masterfrom
Ser9ei:watchcat

Conversation

@Ser9ei

@Ser9ei Ser9ei commented Aug 10, 2026

Copy link
Copy Markdown

Adds a checkbox to enable/disable each watchcat instance.

Pull request details

Description

Adds a checkbox to enable/disable each watchcat instance.
Depends on: openwrt/packages#30234

Maintainer: @jow-


Tested on

OpenWrt Version: 25.12.5
OpenWrt Target/Subtarget: mediatek/filogic
OpenWrt Device: Xiaomi Mi Router AX3000T


Checklist

  • Includes what Issue it closes (e.g. openwrt/luci#issue-number).
  • Includes what it depends on (e.g. openwrt/packages#pr-number in sister repo).

@openwrt-ai openwrt-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed 1 new commit. The commit message matches the diff, and the regenerated po/templates/watchcat.pot is consistent with the JS change (all existing references shifted by +6 lines, the two new msgids inserted in correct sort order at watchcat.js:22/:23). Two inline notes below.


Generated by Claude Code

o = s.taboption('general', form.Flag, 'enabled',
_('Enabled'),
_('Enable or disable this watchcat instance'));
o.default = '1';

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

option enabled only lands in /etc/config/watchcat once the user saves this page — AbstractValue.parse() writes it because rmempty = false, but pre-existing sections that are never re-saved keep no enabled line at all. Current master of the backend never reads the option (config_watchcat() in watchcat.init has no config_get_bool enabled), and the shipped watchcat.config default section has none either.

Does openwrt/packages#30234 use config_get_bool enabled "$1" enabled "1" (i.e. absent == enabled)? If it defaults to 0, every existing installation's watchcat instances would silently stop after the package upgrade. I can't read that PR from this session, so flagging it as a question rather than an assertion.


Generated by Claude Code


o = s.taboption('general', form.Flag, 'enabled',
_('Enabled'),
_('Enable or disable this watchcat instance'));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: every other user-visible string in this file spells the product name capitalised — _('Watchcat') on line 11 and "you can have Watchcat restart your ModemManager interface" on [line 111-112](https://github.com/openwrt/luci/blob/9f169b678e037dbb0c6ec1015cc5efba7baa739d/applications/luci-app-watchcat/htdocs/luci-static/resources/view/watchcat.js#L111-L112).`` Lowercase watchcat here is inconsistent for translators.

Suggested change
_('Enable or disable this watchcat instance'));
_('Enable or disable this Watchcat instance'));

Changing the msgid means po/templates/watchcat.pot needs regenerating so its entry matches.


Generated by Claude Code

Adds a checkbox to enable/disable each watchcat instance.

Signed-off-by: Sergei S. <it4notice@proton.me>

@openwrt-ai openwrt-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed 1 new commit; no new issues found.

The amended commit only capitalises Watchcat in the new help string and regenerates the matching po/templates/watchcat.pot msgid — the previous nit is resolved and the .pot stays consistent (alphabetical position unchanged, all line references still correct). Commit message still matches the diff, FormalityCheck is green.

The open thread about the backend consumer for option enabled (openwrt/packages#30234) is unchanged and still applies.


Generated by Claude Code

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