JSON-RPC: Add client method to set instrument

[hoffie]

Raised by @softins in https://github.com/jamulussoftware/jamulus/pull/1975#pullrequestreview-901000643

I must admit I struggle to understand a reason for making skillLevel changeable via the RPC, but not an instrument. I would only ever need to change my skill level if I was also changing my instrument, as my skill level on a given instrument is pretty static! [...] we need to be able to set the user's instrument. In fact, the complement to jamulusclient/getChannelInfo maybe should be jamulusclient/setChannelInfo for consistency?

Has this feature been discussed and generally agreed?

I assume agreement that a way for setting the instrument should be added. There's no agreement on the particular implementation yet.

Describe the solution you'd like

1. Add jamulusclient/setInstrument and keep setName and setSkillLevel

• :+1: Most fine-grained solution
• :+1: Saves the API user from having to request and (re)set all existing, unchanged values
• :-1: Amount of code

2. Add jamulusclient/setChannelInfo and drop setName and setSkillLevel

• :+1: Consistent with getChannelInfo
• :+1: Uses a single protocol message instead of one per change
• :-1: Requires API users to request the current data first and pre-fill the fields which should not be changed. Maybe we could make the fields optional, though?

I'd go with (2). I read @pljones' https://github.com/jamulussoftware/jamulus/pull/1975#issuecomment-1059839552 as also supporting that approach. Edit: Based on the feedback below, (2) is the generally agreed approach.

[ann0see]

Requires API users to request the current data first and pre-fill the fields which should not be changed. Maybe we could make the fields optional, though?

Yes. Assuming that non existent != setting the respective field to the empty string.

[softins]

Requires API users to request the current data first and pre-fill the fields which should not be changed. Maybe we could make the fields optional, though?

Yes. Assuming that non existent != setting the respective field to the empty string.

I would agree. Allow a subset of the params to be given, and only change the ones that were given.

[ann0see]

I think we have an agreement here.

So this issue should mark (2) as agreed.

[pljones]

I would agree. Allow a subset of the params to be given, and only change the ones that were given.

We'd have to keep it possible to "clear" the value of a field.

[softins]

I would agree. Allow a subset of the params to be given, and only change the ones that were given.

We'd have to keep it possible to "clear" the value of a field.

Absolutely. The request to do that would need to include "field": "" or "field": null as appropriate.

[ann0see]

@softins @hoffie any updates here?

[pljones]

I'm going to be trying to split out all the "moving to client (and server) what should be in client (and server) rather than UI" code from my settings stuff at some point (but after patch/server-listfilter-logging as it will part of refactoring feature/next-big-thing-for-settings into consumable chunks).

I also think @pgScorpio has some changes that may require exposing these "client" operations.

I'd suggest a plan to do them on a "protocol message at a time" basis - i.e. separate PRs for each change covering one message.

[pljones]

Moving to 3.9.1. Hopefully more JSON-RPC progress can be made.

[pljones]

Bumping as notabug.

[pljones]

Removing the milestone and moving to Triage as no one seems interested in picking it up.