updating pointer with a function

fluxia · November 19, 2018, 9:35pm

Hello

I am trying to write a piece of code to update a pointer with a function.

The function I am using is scalePointerUpdate. I have pasted below part of the code I am testing (but I can repost with more details if needed). The problem I have is that although the "currentSelection" variable updates correctly and is printed correctly, the pointer does not change when this variable changes. If I comment out "scalePointerUpdate (currentSelection);" line in the EnterSelection function i get the chrom scale printed out, otherwise I get the minor scale printed out.

Any advice?

Thanks

const int scaleLenght = 8;
int chrom[scaleLenght] = { 0, 1, 2, 3, 4, 5, 6, 7 };
int minor[scaleLenght] = { 0, 2, 4, 5, 7, 9, 11, 12 };
int major[scaleLenght] = { 0, 2, 3, 5, 7, 8, 10, 12 };

int *activeScalePointer = chrom;

enum ScaleMusicali {
	CHROM,
	MINOR,
	MAJOR
};

void scalePointerUpdate (enum ScaleMusicali currentSelection) {
	Serial.println (currentSelection);
	switch (currentSelection) {
	case CHROM: activeScalePointer = chrom;
	case MAJOR: activeScalePointer = major;
	case MINOR: activeScalePointer = minor;
	}
}

void enterSelection () {
	currentSelection = presetSelection;
	currentSelectionPrint = getCurrentSelectionName (currentSelection);
	scalePointerUpdate (currentSelection);
	menu.update ();
}
void printScale () {
	for (int i = 0; i < scaleLenght; i++) {
		Serial.print (*activeScalePointer);
		activeScalePointer++;
		Serial.print (" ");
	}
	Serial.println ();
	scalePointerUpdate (currentSelection);
}

gfvalvo · November 19, 2018, 9:49pm

Please post enough code so that it actually compiles.

RayLivingston · November 20, 2018, 12:07am

fluxia:
Any advice?

Yes. Learn the correct syntax for a switch statement....

Regards,
Ray L.

Blue_Eyes · November 20, 2018, 12:24am

There should be break statements within the switch, otherwise all cases beginning with the match will be executed.

  switch (currentSelection) {
    case CHROM: activeScalePointer = chrom; break;
    case MAJOR: activeScalePointer = major; break;
    case MINOR: activeScalePointer = minor; break;
    default: break;

  }

fluxia · November 20, 2018, 12:25am

Thank you! and yes clearly I still have a lot to learn, but that's the fun of it. I have included break; to each statement and it works.
I have actually used the switch statement for this function in the same way I did it for the other functions "getPresetSelectionName " and "getCurrentSelectionName " but for these function it works fine.
I learnt it from a post on the web suggesting to do this:

const char* getDayName(enum Days day) 
{
   switch (day) 
   {
      case Sunday: return "Sunday";
      case Monday: return "Monday";
      /* etc... */
   }
}

Why is it ok to do it in some case and not in others? I think I have my answer to this, in the first two functions for each case it returns a value, hence it exit the block. (?)

PS
Also I am not sure I understand why the type of this function is a constant. :-[

DKWatson · November 20, 2018, 12:42am

There are instances where a fall-through in a switch statement is desired. The results, if not intentionally coded, should only be considered lucky if they worked. The last assignment will take precedence.

An example of programmed fall-through is typical in command parsing;

case 'A':
case 'a':
// do stuff if 'A' or 'a' is detected
break;

And for your last question, it's not the function that's a constant, it's the return value. Why? Because that's the way whoever wrote the code decided the value should be treated. As the input to the function is an enum and enums are constants, it's likely that the return value is being used somewhere that requires a constant.

gfvalvo · November 20, 2018, 12:42am

fluxia:
Why is it ok to do it in some case and not in others? I think I have my answer to this, in the first two functions for each case it returns a value, hence it exit the block. (?)

Yes, the 'return' statement prevents the 'case fall-through' (hint: you might want to Google that term).

Also I am not sure I understand why the type of this function is a constant. :-[

Because the function is returning a pointer to a c-string literal. It would be bad form to modify it via the pointer.

fluxia · November 20, 2018, 12:57am

Thank you very much to all of you! Much appreciated!

fluxia · November 20, 2018, 7:13pm

Hi again , I have read a bit about the c-string pointers and I suppose that in my case it would make more sense to use a non-const char array ... maybe...

I am posting my code below to explain better. I am basically using this pointer to update my LCD menu so the outcome of function getPresetSelectionName will update my string array presetSelectionPrint .

The code works fine but I wander if I am doing something weird because if I paste the code in Visual Studio I get a message that I cannot initialize a char* with a const char* - which obviously makes sense (and I also get messages about assigning INT to entities of the type created with the enum in the function "increaseSelection").

enum ScaleMusicali {
  CHROM,
  MINOR,
  MAJOR
};

const char* getPresetSelectionName (enum ScaleMusicali presetSelection) {
  switch (presetSelection) {
    case CHROM: return "Chromatic";
    case MAJOR: return "Major";
    case MINOR: return "Minor";
  }
}

char* presetSelectionPrint = getPresetSelectionName (presetSelection);

void increaseSelection () {
  presetSelection = presetSelection + 1;
  if (presetSelection > 2) {
    presetSelection = 0;
  }
  presetSelectionPrint = getPresetSelectionName (presetSelection);
}

gfvalvo · November 20, 2018, 7:41pm

See Reply #1.

fluxia · November 20, 2018, 8:29pm

sorry.. here it is. I have taken out all the LCD stuff and commented out the parts which need the button library and it now compiles without any library needed

//#include "Button.h"
//
//// Button objects instantiation
//const bool pullup = true;
//Button up (2, pullup, 10);
//Button down (3, pullup, 10);
//Button enter (4, pullup, 10);

const int scaleLenght = 8;
int chrom[scaleLenght] = { 0, 1, 2, 3, 4, 5, 6, 7 };
int minor[scaleLenght] = { 0, 2, 4, 5, 7, 9, 11, 12 };
int major[scaleLenght] = { 0, 2, 3, 5, 7, 8, 10, 12 };

int *activeScalePointer = chrom;

enum ScaleMusicali {
  CHROM,
  MINOR,
  MAJOR
};

ScaleMusicali currentSelection = CHROM;
ScaleMusicali presetSelection = CHROM;

const char* getPresetSelectionName (enum ScaleMusicali presetSelection) {
  switch (presetSelection) {
    case CHROM: return "Chromatic";
    case MAJOR: return "Major";
    case MINOR: return "Minor";
  }
}
const char* getCurrentSelectionName (enum ScaleMusicali currentSelection) {
  switch (currentSelection) {
    case CHROM: return "Chromatic";
    case MAJOR: return "Major";
    case MINOR: return "Minor";
  }
}

char* presetSelectionPrint = getPresetSelectionName (presetSelection);
char* currentSelectionPrint = getCurrentSelectionName (currentSelection);


void scalePointerUpdate (enum ScaleMusicali currentSelection) {
  Serial.println(currentSelection);
  switch (currentSelection) {
    case CHROM: activeScalePointer = chrom;
      break;
    case MAJOR: activeScalePointer = major;
      break;
    case MINOR: activeScalePointer = minor;
      break;
  }
}

void increaseSelection () {
  presetSelection = presetSelection + 1;
  if (presetSelection > 2) {
    presetSelection = 0;
  }
  presetSelectionPrint = getPresetSelectionName (presetSelection);
}
void decreaseSelection () {
  presetSelection = presetSelection - 1;
  if (presetSelection < 0) {
    presetSelection = 2;
  }
  presetSelectionPrint = getPresetSelectionName (presetSelection);
}

void enterSelection () {
  currentSelection = presetSelection;
  currentSelectionPrint = getCurrentSelectionName (currentSelection);
  scalePointerUpdate (currentSelection);
}

void printScale () {
  for (int i = 0; i < scaleLenght; i++) {
    Serial.print (*activeScalePointer);
    activeScalePointer++;
    Serial.print (" ");
  }
  Serial.println ();
  scalePointerUpdate (currentSelection);
}

void setup () {

  Serial.begin (9600);

  printScale ();
  activeScalePointer = major;
  printScale ();
  activeScalePointer = minor;
  printScale ();
  activeScalePointer = chrom;
}

void loop () {

  //  if (up.check () == LOW) {
  //    Serial.println ();
  //    Serial.println ("UP pressed");
  //    increaseSelection ();
  //  }
  //
  //  if (down.check () == LOW) {
  //    Serial.println ();
  //    Serial.println ("DOWN pressed");
  //    decreaseSelection ();
  //  }
  //
  //  if (enter.check () == LOW) {
  //    Serial.println ();
  //    Serial.println ("ENTER pressed");
  //    enterSelection ();
  //    printScale ();
  //  }

}

system · November 21, 2018, 9:17am

Why do you have two functions that do EXACTLY the same thing?

void printScale () {
  for (int i = 0; i < scaleLenght; i++) {
    Serial.print (*activeScalePointer);
    activeScalePointer++;
    Serial.print (" ");
  }
  Serial.println ();
  scalePointerUpdate (currentSelection);
}

Anonymous printing sucks.

  printScale ();
  activeScalePointer = major;
  printScale ();
  activeScalePointer = minor;
  printScale ();
  activeScalePointer = chrom;

Print some data with activeScalePointer pointing who know where, then make it point some place specific. I'll never understand people that do this.

What IS the problem?

fluxia · November 21, 2018, 6:09pm

Thanks for your reply. Maybe I should have clarified that I am not concerned about all the print stuff in my code. That's just for me to monitor what's going on and I will remove it in my final code so I am not bothered if it is not elegantly written.

The code works as intended so there is no problem. I was just trying to clarify two issues:

1 - whether I should declare getPresetSelectionName as "const char*" or "char*"; they both seem to work fine.
2 - if it is ok to treat ENUM types as INTin the way I do (i.e.:

	presetSelection = presetSelection - 1;
	if (presetSelection < 0) {
		presetSelection = 2;
	}

... which also works but Visual Studio marks it as an errors: "a value of type INT cannot be assigned to an entity of type "ScaleMusicali".

I just want to learn it the right way.

gfvalvo · November 22, 2018, 3:08am

When in doubt, your first step should be to look to the compiler. Are you upsetting it? Your code produces warnings. If you try to compile it for a Teensy 3.2, it will throw errors. I guess different board packages have different levels of compiler tolerance specified.

I already told you in Reply #6 why it's a bad idea to assign a pointer to a c-string literal to a 'char *' pointer variable.

BTW, are you aware that these two lines are outside of any function? They are initializations and will only be applied once:

char* presetSelectionPrint = getPresetSelectionName (presetSelection);
char* currentSelectionPrint = getCurrentSelectionName (currentSelection);

Regarding your playing fast and loose with enum types, if you know what you're doing, you can cast it:

presetSelection = (ScaleMusicali)(presetSelection + 1);

Also, instead of this:

presetSelection = 2;

why not this:

presetSelection = MAJOR;

You also need a default clause / action at the end of your 'switch (presetSelection) {' constructs.

So, unless you know enough to safely ignore these compiler warnings, you should fix them.

fluxia · November 22, 2018, 9:44pm

Great !
Thank you very much for your advice.

Topic		Replies	Views
Updating Class Variables with Switch Case Programming Questions	7	1257	May 5, 2021
Making a library to increase a function with analog value Programming Questions	4	290	January 15, 2022
Changing pointer within function Programming Questions	3	536	May 5, 2021
Pointers and Arrays: How to correctly add values to an array using a function Sensors	6	1314	May 6, 2021
Update variable passed as parameter to a function Programming Questions	15	2784	May 5, 2021

updating pointer with a function

Related Topics