lang: add Bela ServerOption only on Bela builds

[elgiano]

Purpose and Motivation

Fixe #82

Types of changes

• Documentation
• Bug fix
• New feature

To-do list

• [ ] Code is tested
• [ ] All tests are passing
• [ ] Updated documentation
• [ ] This PR is ready for review

[mossheim]

i don't see why a new class is needed, it's not a big deal if that code also resides in the ServerOptions class

[elgiano]

I tend to think that it's preferrable to keep Bela-specific code on its own... it makes me feel like further work on it will be easier this way. But I'm also open to intersperse them around when possible, and actually I realize that what are now Bela-specific options could become a set of standard ServerOptions for other embedded platforms in the future..

So, I'm up for ditching BelaServerOptions and give its functionalities to ServerOptions. Shall I also just add a primitive to Platform (for instance), instead of creating a Bela-specific primitive .cpp file?

[giuliomoro]

merged 631de735d77d1abb7567b9393b2a2fddadd19c46

[giuliomoro]

Actually @elgiano your latest commit was not working:

• BELA was not defined for sclang
• var o was not declared

I supersquashed everything in bela-supersquash-rebased and added a commit on top that makes it build and the class library compile, but it fails at runtime with

^^ ERROR: Message 'numAnalogInChannels' not understood.

can you have a look?

[elgiano]

of course, I'll have a look now! Strange, I remember I tried it and it was working! Something wrong has happened somewhere :) Taking a look now

[elgiano]

sorry! there was a discrepancy between changes I did on my Bela and the repo on my host, which is the one I push.... this patch should fix it, I'm rebuilding now to see if it actually works!

diff --git a/SCClassLibrary/Common/Control/BelaServerOptions.sc b/SCClassLibrary/Common/Control/BelaServerOptions.sc
index ae83f9cfa..1a8a52aa8 100644
--- a/SCClassLibrary/Common/Control/BelaServerOptions.sc
+++ b/SCClassLibrary/Common/Control/BelaServerOptions.sc
@@ -22,18 +22,18 @@ BelaServerOptions {
    }

    *asOptionsString { | opts |
-       var o = " -J " ++ this.getOpt(\numAnalogInChannels);
-       o = o ++ " -K " ++ this.getOpt(\numAnalogOutChannels);
-       o = o ++ " -G " ++ this.getOpt(\numDigitalChannels);
-       o = o ++ " -Q " ++ this.getOpt(\headphoneLevel);
-       o = o ++ " -X " ++ this.getOpt(\pgaGainLeft);
-       o = o ++ " -Y " ++ this.getOpt(\pgaGainRight);
-       o = o ++ " -A " ++ this.getOpt(\speakerMuted);
-       o = o ++ " -x " ++ this.getOpt(\dacLevel);
-       o = o ++ " -y " ++ this.getOpt(\adcLevel);
-       o = o ++ " -g " ++ this.getOpt(\numMultiplexChannels);
-       o = o ++ " -T " ++ this.getOpt(\belaPRU);
-       o = o ++ " -E " ++ this.getOpt(\belaMaxScopeChannels);
+       opts = " -J " ++ this.getOpt(\numAnalogInChannels);
+       opts = opts ++ " -K " ++ this.getOpt(\numAnalogOutChannels);
+       opts = opts ++ " -G " ++ this.getOpt(\numDigitalChannels);
+       opts = opts ++ " -Q " ++ this.getOpt(\headphoneLevel);
+       opts = opts ++ " -X " ++ this.getOpt(\pgaGainLeft);
+       opts = opts ++ " -Y " ++ this.getOpt(\pgaGainRight);
+       opts = opts ++ " -A " ++ this.getOpt(\speakerMuted);
+       opts = opts ++ " -x " ++ this.getOpt(\dacLevel);
+       opts = opts ++ " -y " ++ this.getOpt(\adcLevel);
+       opts = opts ++ " -g " ++ this.getOpt(\numMultiplexChannels);
+       opts = opts ++ " -T " ++ this.getOpt(\belaPRU);
+       opts = opts ++ " -E " ++ this.getOpt(\belaMaxScopeChannels);
    }

    *getOpt { |serverOptions, optionName|

[elgiano]

Ok, no, sorry I made a mess here. I'll take a little time to fix it.

[elgiano]

Here we go, this patch for bela-supersquash-rebased should fix it. I tested it and it works on my Bela. I've reset the repo on my Bela to bela-supersquash-rebased HEAD, then applied the patch, built, installed, tested, and it worked :)

Here is the patch:

diff --git a/SCClassLibrary/Common/Control/BelaServerOptions.sc b/SCClassLibrary/Common/Control/BelaServerOptions.sc
index ae83f9cfa..421e0d472 100644
--- a/SCClassLibrary/Common/Control/BelaServerOptions.sc
+++ b/SCClassLibrary/Common/Control/BelaServerOptions.sc
@@ -2,7 +2,7 @@ BelaServerOptions {

    classvar defaultValues;

-   *initClass {
+   *initDefaults {
        defaultValues = IdentityDictionary.newFrom(
            (
                numAnalogInChannels: 2,
@@ -18,26 +18,38 @@ BelaServerOptions {
                belaPRU: 1,
                belaMaxScopeChannels: 0
            )
-       )
+       );
+       ^defaultValues;
    }

-   *asOptionsString { | opts |
-       var o = " -J " ++ this.getOpt(\numAnalogInChannels);
-       o = o ++ " -K " ++ this.getOpt(\numAnalogOutChannels);
-       o = o ++ " -G " ++ this.getOpt(\numDigitalChannels);
-       o = o ++ " -Q " ++ this.getOpt(\headphoneLevel);
-       o = o ++ " -X " ++ this.getOpt(\pgaGainLeft);
-       o = o ++ " -Y " ++ this.getOpt(\pgaGainRight);
-       o = o ++ " -A " ++ this.getOpt(\speakerMuted);
-       o = o ++ " -x " ++ this.getOpt(\dacLevel);
-       o = o ++ " -y " ++ this.getOpt(\adcLevel);
-       o = o ++ " -g " ++ this.getOpt(\numMultiplexChannels);
-       o = o ++ " -T " ++ this.getOpt(\belaPRU);
-       o = o ++ " -E " ++ this.getOpt(\belaMaxScopeChannels);
+   *addBelaOptions { |serverOptions|
+       var belaOpts = (defaultValues ?? { this.initDefaults }).copy;
+       belaOpts.keys.do { |optName|
+           serverOptions.addUniqueMethod(optName) { belaOpts[optName] };
+           serverOptions.addUniqueMethod(optName.asSetter) { |self, newValue|
+               belaOpts[optName] = newValue
+           };
+       }
+   }
+
+   *asOptionsString { |serverOptions|
+       var o = " -J " ++ this.getOpt(serverOptions, \numAnalogInChannels);
+       o = o ++ " -K " ++ this.getOpt(serverOptions, \numAnalogOutChannels);
+       o = o ++ " -G " ++ this.getOpt(serverOptions, \numDigitalChannels);
+       o = o ++ " -Q " ++ this.getOpt(serverOptions, \headphoneLevel);
+       o = o ++ " -X " ++ this.getOpt(serverOptions, \pgaGainLeft);
+       o = o ++ " -Y " ++ this.getOpt(serverOptions, \pgaGainRight);
+       o = o ++ " -A " ++ this.getOpt(serverOptions, \speakerMuted);
+       o = o ++ " -x " ++ this.getOpt(serverOptions, \dacLevel);
+       o = o ++ " -y " ++ this.getOpt(serverOptions, \adcLevel);
+       o = o ++ " -g " ++ this.getOpt(serverOptions, \numMultiplexChannels);
+       o = o ++ " -T " ++ this.getOpt(serverOptions, \belaPRU);
+       o = o ++ " -E " ++ this.getOpt(serverOptions, \belaMaxScopeChannels);
+       ^o;
    }

    *getOpt { |serverOptions, optionName|
-       serverOptions.instVarGet(optionName) ? this.defaultValues[optionName];
+       ^(serverOptions.perform(optionName) ? defaultValues[optionName]);
    }

 }
diff --git a/SCClassLibrary/Common/Control/Server.sc b/SCClassLibrary/Common/Control/Server.sc
index d34ed9f05..e5b1ba90f 100644
--- a/SCClassLibrary/Common/Control/Server.sc
+++ b/SCClassLibrary/Common/Control/Server.sc
@@ -58,20 +58,6 @@ ServerOptions {

    var <>safetyClipThreshold;

-   // extension for BELA
-   var <>numAnalogInChannels;
-   var <>numAnalogOutChannels;
-   var <>numDigitalChannels;
-   var <>headphoneLevel;
-   var <>pgaGainLeft;
-   var <>pgaGainRight;
-   var <>speakerMuted;
-   var <>dacLevel;
-   var <>adcLevel;
-   var <>numMultiplexChannels;
-   var <>belaPRU;
-   var <>belaMaxScopeChannels;
-
    *initClass {
        defaultValues = IdentityDictionary.newFrom(
            (
@@ -115,18 +101,6 @@ ServerOptions {
                recBufSize: nil,
                bindAddress: "127.0.0.1",
                safetyClipThreshold: 1.26 // ca. 2 dB
-               numAnalogInChannels: 2,
-               numAnalogOutChannels: 2,
-               numDigitalChannels: 16,
-               headphoneLevel: -6,
-               pgaGainLeft: 10,
-               pgaGainRight: 10,
-               speakerMuted: 0,
-               dacLevel: 0,
-               adcLevel: 0,
-               numMultiplexChannels: 0,
-               belaPRU: 1,
-               belaMaxScopeChannels: 0,
            )
        )
    }
@@ -136,7 +110,10 @@ ServerOptions {
    }

    init {
-       defaultValues.keysValuesDo { |key, val| this.instVarPut(key, val) }
+       defaultValues.keysValuesDo { |key, val| this.instVarPut(key, val) };
+       if (Platform.hasBelaSupport, {
+           BelaServerOptions.addBelaOptions(this)
+       });
    }

    device {

I removed bela-specific options from ServerOptions defaults and instance variables. Everything is managed by BelaServerOptions, which now "adds" those instance variables at runtime (with a mechanism called addUniqueMethods), so that we get better encapsulation and still the usual interface s.options.numAnalogOutChannels = ....

And here the script I used to check that everything worked:

s = Server.default;
s.options.maxLogins = 4;
s.options.numAnalogInChannels = 8;
s.options.numAnalogOutChannels = 0;
s.options.numDigitalChannels = 16;
s.options.bindAddress = "0.0.0.0";

s.options.blockSize = 16;
s.options.numInputBusChannels = 2;
s.options.numOutputBusChannels = 2;
s.options.belaMaxScopeChannels = 4;
s.options.asOptionsString.postln;
s.waitForBoot({
    {SoundIn.ar(0).belaScope(0) * 10}.play
});

It prints the option string that will be used to start the server. I looked at it reflects custom and default values.

[giuliomoro]

thanks, this seems to work well.

Is it needed to have the default values both in Server.sc and BelaServerOptions.sc ? I feel like the duplication would be better avoided if possible ...

so that we get better encapsulation and still the usual interface s.options.numAnalogOutChannels = ....

I was thinking of prepending bela to all Bela options, to create a pseudo-namespace, would it then make sense then to have something likes.options.Bela.numAnalogIn (etc) instead?

[elgiano]

Is it needed to have the default values both in Server.sc and BelaServerOptions.sc ? I feel like the duplication would be better avoided if possible ...

Doesn't my patch remove them from Server.sc already?

s.options.Bela.numAnalogIn

I think this is more something to be discussed in main SuperCollider after the merge, especially if multiple "special" backends like Bela appear. In that case I think it would be nice to have separate interfaces, like the one you propose, separate fields within ServerOptions. But for now, since it's only Bela, I think it's enough to have it like it is (separate class/source file, but options merged in the same ServerOptions object). And of course I'm open to what other people think about it, and it would be easy to implement options.bela.

[giuliomoro]

Doesn't my patch remove them from Server.sc already?

Yes you are right. I must have been confused!

OK I'll go ahead making a PR upstream. Thanks for your help and fingers crossed!

[elgiano]

Nice!!! Looking forward, it's been a pleasure so far and thanks for your work!