Return "ok" from setSkillLevel when skillLevel is null

[mcfnord]

🤖 AI: Fixes #3919, assigned by @pljones for 4.1.0. jamulusclient/setSkillLevel documents null as a valid value that clears the skill level, and documents its result as always "ok". The null branch clears the level and returns without setting response["result"], so the reply carries neither result nor error, which the JSON-RPC 2.0 response object forbids. A client that waits for result waits forever on a valid call.

Short description of changes

One line: response["result"] = "ok"; before the return in the null branch, the same statement setName and the string branch of this method already end with.

Both arms built from the same tree with and without this line, Qt 5.15.13, gcc 13.3, x86-64, client headless (-n -i /dev/null, QT_QPA_PLATFORM=offscreen), raw reply lines from the RPC socket:

skillLevel	main	with this change
null	{"id":1,"jsonrpc":"2.0"}	{"id":1,"jsonrpc":"2.0","result":"ok"}
"expert"	{"id":2,"jsonrpc":"2.0","result":"ok"}	same
42	-32602, Invalid params: skillLevel is not a string	same
"guru"	-32602, Invalid params: skillLevel is not beginner, intermediate or expert	same

CHANGELOG: Client: jamulusclient/setSkillLevel now returns "ok" when called with null, instead of a JSON-RPC response with neither result nor error.

Context: Fixes an issue?

Fixes: #3919

Does this change need documentation? What needs to be documented and how?

No. The documented behaviour is what the code now does.

Status of this Pull Request

Ready for review.

What is missing until this pull request can be merged?

Review.

Checklist

• I've verified that this Pull Request follows the general code principles
• I tested my code and it does what I want — evidence above
• My code follows the style guide
• I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
• I've filled all the content above

---

🤖 This message was written by AI and reviewed by @mcfnord.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: QUIET

Plan: Advanced

Run ID: 02824d4e-87cb-4024-a9ef-7f51edcf7469

📥 Commits

Reviewing files that changed from the base of the PR and between c002c51 and 1f3aecd.

📒 Files selected for processing (1)

• src/clientrpc.cpp

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

The jamulusclient/setSkillLevel null path now resets the skill level and returns "ok" in the JSON-RPC result.

Changes

Skill level reset response

Layer / File(s)	Summary
JSON-RPC success result src/clientrpc.cpp	The null skill-level branch now sets the response result to "ok" before returning.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 1f3ae

Clearing a client skill level now returns the expected JSON-RPC "ok" result, preventing malformed success responses. The focused change is ready to merge.

Suggested reviewers: dingodoppelt

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ 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%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The change satisfies issue #3919 by returning "ok" for null, preserving the skill-level reset and SetRemoteInfo() call, and preventing an invalid JSON-RPC response without result or error.
Out of Scope Changes check	✅ Passed	The pull request changes only the null branch of jamulusclient/setSkillLevel and does not include unrelated code changes.
Title check	✅ Passed	The title clearly and concisely describes the main change: returning "ok" from setSkillLevel when skillLevel is null.
Description check	✅ Passed	The description follows the repository template, explains the issue and implementation, documents testing results, states documentation needs, identifies the linked issue, and completes the checklist.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[pljones]

Not going to say "no" to this...