watchcat: add per-instance 'enabled' option (default 1)

[Ser9ei]

Allows disabling individual watchcat instances without removing them.

📦 Package Details

Maintainer: @danielfdickinson & @dhrm1k

Description:
The new "enabled" option defaults to "1", so existing configurations
continue to work without any changes. No config migration is required.

Related luci PR: openwrt/luci#8930

---

🧪 Run Testing Details

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

---

✅ Formalities

• I have reviewed the CONTRIBUTING.md file for detailed contributing guidelines.

[dhrm1k]

Thanks. Defaulting enabled to 1 preserves the
behaviour of existing configurations.

Could you also add the new option to the default watchcat.config,
as a commented example?

# option enabled '1'

[Ser9ei]

as a commented example?

# option enabled '1'

good point, done.
@dhrm1k, could you please approve the changes if you're ok with this PR?

[dhrm1k]

LGTM! thanks for your contribution.

[danielfdickinson]

Also LGTM. Let's see what the automated review says tonight.

[openwrt-ai]

Reviewed 1 commit. Nothing here blocks a merge.

The change looks correct: enabled defaults to 1 so existing configs are unaffected, the config_get_bool <name> "$1" enabled 1 pattern matches the in-tree convention, the early return runs before any instance is opened, and PKG_RELEASE is bumped 25 → 26 as required for an init-script change with no PKG_VERSION change. The two inline comments are both optional — one cosmetic quoting nit, one pre-existing bug in the same function that this PR does not cause.

---

Generated by Claude Code

[openwrt-ai]

nit (optional, cosmetic): every other default in this function is quoted (config_get_bool unlockbands "$1" unlockbands "0", config_get period "$1" period "120"); this one is bare. The option name and the config_get_bool ... 1 fallback otherwise match the in-tree convention (e.g. uhttpd.init:117, trelay.init:7), so this is purely about matching the surrounding lines.

Suggested change

	config_get_bool enabled "$1" enabled 1
	config_get_bool enabled "$1" enabled "1"

---

Generated by Claude Code

[openwrt-ai]

The early return itself is correct: config_get_bool normalises to 0/1 (or falls back to the default for garbage input), so the -eq comparison can't blow up on a hand-edited value, and since start_service re-declares the whole instance set, procd_close_service "set" tears down an instance that stops being declared — so flipping enabled to 0 and reloading does stop the running instance.

Pre-existing, not introduced here, but adjacent enough to be worth mentioning while this function is open: error and warn are plain globals that are never cleared between sections. append_string "error" at watchcat.init:58 accumulates into them, and the [ -n "$error" ] guard at watchcat.init:95-99 returns early, so with two or more config watchcat sections a validation failure in the first one silently suppresses every later section too (and re-logs the first section's message under each subsequent section name). Clearing them at the top of config_watchcat would fix it. Entirely optional for this PR — happy to see it as a separate change.

---

Generated by Claude Code

[dhrm1k]

@Ser9ei look into what automated review says

[Ser9ei]

@Ser9ei look into what automated review says

"Nothing here blocks a merge."

---

PS.

To my mind, this looks clearly outside the scope of this PR.

It would be better to continue the code review in a separate issue and consider a retrospective review of the existing watchcat.init, especially since this is an old package and the AI review has already found one possible pre-existing bug.

[danielfdickinson]

@Ser9ei A couple of things.

1. Since we have to wait for someone with commit rights anyway (both @dhrm1k and myself @danielfdickinson are maintainers, but only a small number of people have actual commit rights, and we are not in that group), could you fix the 'nit' from the first part of the automated review?
2. I agree that the pre-existing bug should be handled in a separate review. I started work on a retrospective review of the watchcat scripts, but have had rather a number of other more urgent matters (bit in and outside OpenWrt participation).

[Ser9ei]

This looks like the same issue. It would make more sense to review the existing quoting style in watchcat.init and clean it up where appropriate.

[danielfdickinson]

@Ser9ei I think the second on is a potential logic issue that should be a separate PR. The first (the nit) is about matching this package's style convention.

Whether the package as a whole should be updated to another convention (unquoted numbers) would be a topic for a separate PR, but I do not think adding mixed styles in this package is justified by the potential future PR.

That is, mixed styles would be a new issue while the other is old.

[Ser9ei]

The unquoted numeric default is consistent with OpenWrt's documented config_get_bool usage and with the convention used in OpenWrt's core init.d scripts.
A broader quoting cleanup of watchcat.init would be better handled separately.

match the in-tree convention (e.g. uhttpd.init:117, trelay.init:7)

doc: https://openwrt.org/docs/guide-developer/config-scripting?s[]=single&s[]=set&s[]=shell

[danielfdickinson]

@Ser9ei I'm not arguing that the quoted style is what should stay forever, I am saying that since we are not handling a review of of the entire package (including style differences from the main repo) and that at present the function uses quotes, that this PR should use quotes, and any 'fix' of the function / script as a whole should be handled as a separate PR. That is, please be consistent with the current reality for the function and leave any 'corrections' (unquotedness) to a separate PR.

To put it another way, mixing styles is worse, to my mind, than not following the suggested style guidelines (which have not been treated as hard requirements for many years, and there are many packages with idiosyncrasies to be managed) for one new line.

Also to quote the llm pr review bot prompt (emphasis mine):

Do flag deviations from the existing style of the file being changed or of similar in-tree files — indentation width, brace placement, naming conventions, comment style, etc. The project's style is whatever the existing code does; new code should match it.

So, this new line should match the current function until such time as a cleanup PR of the entire script and/or package is done.