-2

So, I'm using the GinSing library for Arduino, and I've run into a problem. There is a chunk of their code in which they extract a value from a variable name (or object? or something?). I've read up about having values in variable names on here (Stack Overflow) and I know that you're not supposed to do it, but I'm stuck!

I'm not a good enough programmer to modify their code, but I still want to use it (the GinSing shield is pretty cool). Here's an example of the usage of their s->setEnvelope:

s->setEnvelope (OSC_1, AT_100MS, 1.0f, DR_100MS, 1.0f, DR_100MS, 0.0f);

I want to randomly change those values so I made this:

String adsrMake(String type, int attack){
    return type + attack + "MS";
}

and then I do this:

 s->setEnvelope (OSC_1, adsrMake("AT_",time/2),  etc..

but it doesn't like that. It doesn't want a string, it wants a name (?) or something. The error I get says it wants:

void GinSingSynth::setEnvelope(GSSynthOsc, GSAttackDur, float, GSDecRelDur, float, GSDecRelDur, float)

I opened up the .cpp file, and it says it's doing this on the other end:

void GinSingSynth::setEnvelope (GSSynthOsc  oscIdx ,
                                GSAttackDur attackDur , float attackAmp,
                                GSDecRelDur decayDur  , float decayAmp ,
                                GSDecRelDur releaseDur, float releaseAmp )
{
    ubyte voiceIdx = OscIdxToVoiceIdx(oscIdx);

    // Construct ADR bytes ( high four bits amplitude, low four bits duration )

    ubyte atkByte = ( (ubyte) ( 0x0f * attackAmp   ) << 4 ) + attackDur;
    ubyte dcyByte = ( (ubyte) ( 0x0f * decayAmp    ) << 4 ) + decayDur;
    ubyte rlsByte = ( (ubyte) ( 0x0f * releaseAmp  ) << 4 ) + releaseDur;
Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Greg Debicki
  • 235
  • 2
  • 6

3 Answers3

2

It is very likely that GSAttackDur represents some kind of intergal type. it is possibly an enum, so you have to look up the definition and see what the accepted values are. Symbols such as AT_100MS probably represent integral values.

This all means that you could change the values by passing integers like this:

s->setEnvelope (OSC_1, GsAttackDur(42), 1.0f, DR_100MS, 1.0f, DR_100MS, 0.0f);
                                    ^ random                  

however, you must check that the range of valid values are. It is very important that you do not pass an enum value that is not within the accepted range. Unfortunately, GsAttackDur(n) will accept any n, with no regard as to whether it is valid. This means you have to know the valid range, and generate a number n within this range. There was recently a related question which you should check. Bear in mind that your random enum generation will always be coupled to the enum definition. If the latter changes, you will have to modify the former.

Community
  • 1
  • 1
juanchopanza
  • 223,364
  • 34
  • 402
  • 480
  • Ok, awesome. I found this: `// GSAttackDur - ADSR attack durations ( durations are quantized to these values as specified in milliseonds ) enum GSAttackDur { AT_2MS = 0x00, AT_8MS = 0x01, AT_16MS = 0x02, AT_24MS = 0x03, AT_38MS = 0x04, AT_56MS = 0x05, AT_68MS = 0x06, AT_80MS = 0x07, ..etc ` but im still not sure what to do with it. how do swap the numbers when I'm using s->setEnvelope in my code? Do I have to cast my string as something else? or... Sorry, I still don't really understand. Thanks for answering so quickly thought – Greg Debicki May 19 '12 at 09:02
  • @gregdebicki you shouldnt use strings at all. just pass integers corresponding to one of the enum's values. – juanchopanza May 19 '12 at 09:19
  • when i use this i get "invalid conversion from 'int' to 'GSAttackDur'" `s->setEnvelope (OSC_1, 42, 1.0f, DR_100MS, 1.0f, 99, 0.0f); ^ random ^ random` and if i alternate (like this^) it tells me AT_100MS "was not declared in this scope" – Greg Debicki May 19 '12 at 09:19
  • The value is not stored in the name, the numeric string in the symbol name is so that humans know what the value represents - it is just a label. I don't know where you have read about any technique that can extract the symbol name at run time, but it is nonsense. C++ is compiled code, the symbol names are non-existant in the generated machine code. – Clifford May 19 '12 at 09:35
  • 1
    @juanchopanza: Advising to pass integer values seems like a bad idea. You have no way of knowing how some unexpected integer value is going to be interpreted. In this case, the mapping between the integer value and the semantic meaning is completely arbitrary. This function takes `enum` arguments; you should pass only one of the defined `enum` constants. – jamesdlin May 19 '12 at 10:47
  • 1
    @jamesdlin absolutely right. I added some clarification to my answer, as well as a link to a related question. – juanchopanza May 19 '12 at 17:01
  • @GregDebicki I updated my answer with some important information. – juanchopanza May 19 '12 at 17:02
  • @Clifford thanks, i know, i read up on it a lot before asking but because im still pretty new to this stuff it seemed like that's what was going on here so i didn't know how else to phrase my question. @jamesdlin @juanchopanza so i need to pass "0x04" to get 38ms? do i use static_cast to convert between 38ms and 0x04? or are you saying that they're completely unrelated? in that case would i have to set something up like `if(randomNumber == 4){return 0x04;}`? also, side note/question, the syntax for static_cast is `static_cast(theIntIrandomlyGenerated)` right? – Greg Debicki May 19 '12 at 22:06
  • @Greg: 4 is the same as 0x04, merely a different number base notation (decimal vs hexadecimal), so you may as well have just return `static_cast(randomNumber)`, but it is not really a good idea; in fact this answer is not really a good idea for all the reasons mentioned in the answer itself! – Clifford May 20 '12 at 08:25
1
  1. Add their header file to your C/C++ source
  2. Make your function return their struct type or #def probably.

GSAttackDur adsrMake(String type, int attack) { .... }

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Jay D
  • 3,263
  • 4
  • 32
  • 48
1

While you know that the GSAttackDur enum members are sequential numeric values, to rely upon inside information of a third-party library (or even your own library) to infer that a cast of an integer to an enum is valid or safe is not best practice.

It may not always be true in the general case or under code maintenance that the values are sequential or even an arithmetic progression. A better general solution is to use a look-up table to translate your random integer to a valid enum type value, thus requiring no knowledge of the actual enum literal values in either magnitude or order:

GSAttackDur getRandomDuration()
{
    static const GSAttackDur lookup[]
    {
        AT_2MS, AT_8MS, AT_16MS, AT_24MS, AT_38MS, 
        AT_56MS, AT_68MS, AT_80MS, AT_100MS // add more as necessary
    } ;

    // Get random index into lookup - 0 to N-1 where N is the number 
    // of elements in lookup.
    int r = rand() / (RAND_MAX / (sizeof(lookup) / sizeof(*lookup)) ) ;

    // Return a random GSAttackDur value
    return lookup[r] ;
}
Clifford
  • 88,407
  • 13
  • 85
  • 165