0

I'm writing a program where I will be able to toggle between multiple banks, and each bank has 4 presets on it. Right now, I'm trying to give each preset a name that is assigned on creation.

I have an object I called StoredDataManager, which will handle all the banks. In its constructor, I do this:

for (byte i = 0; i < NUMBER_OF_BANKS; i++) {
  Bank bank;
  _banks[i] = &bank;
}

_banks is declared like this: Bank* _banks[NUMBER_OF_BANKS]; in the .hfile of StoredDataManager.

A bank has 4 presets, like this:

Preset* _A;
Preset* _B;
Preset* _C;
Preset* _D;

Which I initialize as this:

Preset preset(presetName, presetData);
...
_A = &preset; ...

Where presetName is "CHR", and presetData doesn't matter, because I'm just trying to get the name to work.

In the preset, the name is saved like this: char _presetName[4];. It is returned like this:

char* Preset::getPresetName() {
  return _presetName;
}

I try to print a preset's name like this:

Serial.println(_banks[0]->getPreset('A')->getPresetName());

But, right now it simply returns .

Why is that? I suspect it has to do with the memory being erased or overwritten.

EDIT: Here is the code of getPreset:

Preset* Bank::getPreset(char presetLocation) {
  switch (presetLocation) {
    case PRESET_LOCATION_1:
      return _A;
    case PRESET_LOCATION_2:
      return _B;
    case PRESET_LOCATION_3:
      return _C;
    case PRESET_LOCATION_4:
      return _D;
    default:
      return NULL;
  }
}

I think now that I'll turn this into 4 separate functions, like getA, getB and so on, but this what it is the moment. PRESET_LOCATION_1 is just a #define PRESET_LOCATION_1 'A'.

EDIT 2: I start in the main arduino .ino file with this:

void setup() {
  Serial.begin(9600);
  StoredDataManager storedDataManager;
}

This triggers this constructor:

StoredDataManager::StoredDataManager() {
  for (byte i = 0; i < NUMBER_OF_BANKS; i++) {
    Bank bank;
    _banks[i] = &bank;
  }
  _currentBankIndex = 0;
  Serial.println(_banks[0]->getPreset('A')->getPresetName());
}

This is _banks: Bank* _banks[NUMBER_OF_BANKS];. The initializing of the bank object runs this constructor: (PRESET_LOCATION_1, 2, 3, and 4 just being 'A', 'B', 'C', 'D')

Bank::Bank() {
  addPreset("CHR", "Some data", PRESET_LOCATION_1);
  addPreset("CHR", "Some data", PRESET_LOCATION_2);
  addPreset("CHR", "Some data", PRESET_LOCATION_3);
  addPreset("CHR", "Some data", PRESET_LOCATION_4);
}

Which calls this function:

void Bank::addPreset(const char* presetName, String presetData, char presetLocaton) {
  Preset preset(presetName, presetData);
  switch (presetLocaton) {
    case PRESET_LOCATION_1:
      _A = &preset; break;
    case PRESET_LOCATION_2:
      _B = &preset; break;
    case PRESET_LOCATION_3:
      _C = &preset; break;
    case PRESET_LOCATION_4:
      _D = &preset; break;
  }
}

Then, above in the StoredDataManages constructor, this line is hit:

Serial.println(_banks[0]->getPreset('A')->getPresetName());

And that's where I try to log out the preset name.

6
  • The use of dynamic memory on a constrained system like avr and the word "safely" as a goal probably do not belong in the same posting. That said, an immediate problem is that 'A' does not mean what you think it does - specifically it is an integer constant of value 65, ie, the ascii code of a capital A You should probably show the code of getPreset() Jan 8, 2017 at 0:41
  • @ChrisStratton I posted the code for getPreset(). I do know that it is the character 'A', though. As I mention at the end there, I'm probably going to change how I get the presets.
    – ptf
    Jan 8, 2017 at 1:02
  • Where do you call preset()? With a pointer to what? That was initialized how? Without including your code, this is unanswerable. Jan 8, 2017 at 1:06
  • @ChrisStratton Tried to explain the flow together with the code now.
    – ptf
    Jan 8, 2017 at 1:17
  • 1
    Just as a style thing, identifiers starting with an underscore and followed by an upper-case letter are reserved by the C++ standard. You should get out of the habit of using them for your own variables.
    – Nick Gammon
    Jan 8, 2017 at 1:18

1 Answer 1

1
void Bank::addPreset(const char* presetName, String presetData, char presetLocaton)
{
  Preset preset(presetName, presetData);

  switch (presetLocaton)
  {
    case PRESET_LOCATION_1:
      _A = &preset; break;
    case PRESET_LOCATION_2:
      _B = &preset; break;
    case PRESET_LOCATION_3:
      _C = &preset; break;
    case PRESET_LOCATION_4:
      _D = &preset; break;
  }
}

I can't see how this is going to work because you are storing somewhere outside a function a pointer to a local variable inside the function. As soon as addPreset exits the variable preset will go out of scope, and a pointer to it will be undefined.


You should probably do a new to make a new instance of Preset which it is then valid to store outside the scope of the function.

4
  • Ah, ok. I was thinking as long as I stored a reference to it, the data at that memory location wouldn't be removed. I'll work on changing that tomorrow. Thanks! Any idea as to how I could create an object that would not be garbage collected that would work in this situation?
    – ptf
    Jan 8, 2017 at 1:23
  • I'll try newing the objects tomorrow. I guess I have the same problem with the banks, then.
    – ptf
    Jan 8, 2017 at 1:26
  • Yes, very much so. Don't assign to some variable a thing that will go out of scope, as you did there. As for garbage collection, there is no default garbage collection in C++.
    – Nick Gammon
    Jan 8, 2017 at 1:42
  • Ah, right. It has been some time since I coded C++ :) Thanks!
    – ptf
    Jan 8, 2017 at 10:22

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.