Add fader level set method to RPC remote interface

[corrados]

Short description of changes

Dear Jamulus development team :-). After a very long time, I worked on Jamulus again. I needed to add a fader level set method to the RPC remote inteface because I want to run Jamulus in headless mode together with my Edrumulus e-drum software on a Raspberry Pi. Edrumulus is remote controlled with a smart phone via a web interface and for Jamulus I want to write a small Android app which uses the RPC interface.

Note: For being able to make changes, I had to create a fork. I remember that this was problematic since I moved the jamulus project from my private space "corrados" to "jamulussoftware" and Github automatically forwarded git actions if a developer did use the old path. Since jamulus is now about 5 years in "jamulussoftware", I hope that in the meantime all developer have updated their Git repo and have cloned from the correct origin URL. If this is not the case, please let me know. Then I will delete my fork again.

CHANGELOG: Added a jamulusclient/setFaderLevel method to the RPC remote interface.

Context: Fixes an issue?

No.

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

Yes, the new method should be documented in the JSON-RPC file. I added Doxygen description to the new function similar to what was done with the other functions. It seems that the documentation file is generated from this. I would need your help how to update the documentation (is it automatically done somehow?).

Status of this Pull Request

I tested my implementation as follows:

Start Jamulus with: ./Jamulus --jsonrpcport 11223 --jsonrpcsecretfile secret.txt
In a Linux shell: nc localhost 11223
Then:
{"id":1,"jsonrpc":"2.0","method":"jamulus/apiAuth","params":{"secret": "my_secret_which_i_do_not_show_here"}}
{"id":1,"jsonrpc":"2.0","method":"jamulusclient/setFaderLevel","params":{"channelIndex": 0, "level": 50}}

That worked for me. The fader of the first channel moved after the last command.

What is missing until this pull request can be merged?

As stated above: The documentation may be updated first.

Checklist

• I've verified that this Pull Request follows the general code principles
• I tested my code and it does what I want
• 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

[corrados]

The Github runner answered one of my questions :-)

"Please run ./tools/generate_json_rpc_docs.py to regenerate docs/JSON-RPC.md"

[pljones]

To keep strict separation of concerns, there should be no direct calls to CClient methods. CClient should have public slots to receive signals for changes that is its concern, such as tracking state.

Ideally, we'd not be adding more code to the existing method of doing it -- the "OnXxxx" method names are more correct, as they imply they're handling signals, which is what they should be doing. So I'm going to object to renaming the method.

[corrados]

Hi Peter, thanks for you comment.

My preferred solution would have be to do it similar as in soundbase: https://github.com/jamulussoftware/jamulus/blob/main/src/sound/soundbase.cpp#L381. But since the RPC class is not a member of the client, I cannot use the signal/slot mechanism. The way it is implementat that the RPC class communicates with the client is with public access functions provided by the client, e.g.: https://github.com/jamulussoftware/jamulus/blob/main/src/clientrpc.cpp#L260.

There is already a similar public function called SetRemoteGain (https://github.com/jamulussoftware/jamulus/blob/main/src/client.h#L258), but this does something slightly different and cannot be used by a "controller in" action.

What I now did was actually not a renaming of an existing function but to introduce a new one. The old function still exists but has a different body: https://github.com/jamulussoftware/jamulus/pull/3571/files#diff-2ee628bb1cdcbf04f484b57ea20ddf172db2dfaeb22214ebfb24ff36718a7509R437

[pljones]

Like I say, it's only the method rename I'm objecting to -- the calls are being used anyway, it's not for this PR to fix.

Wrapping a public method in a public method doesn't really fix the problem, so I'd still say just call the OnXxx method rather than rename it. It'll get reworked at some point in the future.

[corrados]

Ok, I have changed the implementation so that there is just a minimal change in client.h.