[Bug]: Attenuator settings "Vibration" and "Feedback on Overheat" do not survive power cycle

[nomakewan]

What actions were attempted?

1. Access web UI
2. Go to Attenuator Settings
3. Uncheck "Enable Vibration" and "Feedback on Overheat"
4. Click "Update Settings"
5. Power Cycle the unit

What actually happened?

"Enable Vibration" and "Feedback on Overheat" are checked.

What was expected?

"Enable Vibration" and "Feedback on Overheat" should be unchecked.

Firmware Version

5.3.4b

Mode in Use

Any / Both Modes

Notable Hardware

ESP32 Wireless Module/Attenuator

Homework Completed

• [X] I did my part!

[nomakewan]

As more context, I tried to figure this one out myself but couldn't see what would make these two settings any different in the code from, say "Enable Piezo Buzzer." They seem to all be called and interpreted the same way, and yet only these two settings do not respect the change after cycling the power. They do remain untoggled while power remains, but as soon as you restart the ESP32 they show up toggled again.

[DustinGrau]

UGH. Because I'm an idiot. The code which restores the initial values for these options via the ESP's "preferences" system can be given a default if it doesn't find the value. Well, if the value stored is a FALSE, then the default shouldn't be TRUE. This is in the setup() code which is why it's hard to spot.

If I don't specify a default, then the default would be false (0) for the getBool method. And while it may confuse some users we can make a note of this in the operation guide that these audio/physical feedback options will not be enabled by default--you'll need to opt-in to using them. That would allow them to actually be disabled when needed.

[DustinGrau]

Fixed in recent push

[nomakewan]

This did not resolve the issue; it merely reversed the problem on my end. Now instead of those settings remaining stuck on, they remain stuck off. "Enable Piezo Buzzer," which had identical code to the other two, is still respecting user choice as it did before your changes.

[DustinGrau]

You may need to fix it then on your end--I don't have a lot of time to devote to this. The defaults should still stand--if the preference is not there or there's an invalid value, it should default to FALSE. As to why it's not working for you, I would suggest providing a truth table of what you see: set all options off and note what happens, set all on and note what happens. Saying "reversed" doesn't give a clear state of behavior--what was the setting, what was the result, etc.

[DustinGrau]

And yes, the vars will be what the runtime reflects, so the issue is in the saving or recalling of the preferences.

[nomakewan]

You may need to fix it then on your end--I don't have a lot of time to devote to this. The defaults should still stand--if the preference is not there or there's an invalid value, it should default to FALSE. As to why it's not working for you, I would suggest providing a truth table of what you see: set all options off and note what happens, set all on and note what happens. Saying "reversed" doesn't give a clear state of behavior--what was the setting, what was the result, etc.

I actually did this already.

Before your fix, if you set all options to "true," then after rebooting the ESP32 all options would be true. If you set all options to "false," then after rebooting the ESP32 all options would be false except for Enable Vibration and Feedback on Overheat.

After your fix, if you set all options to "true," then after rebooting the ESP32 all options would be true except Enable Vibration and Feedback on Overheat. If you set all options to "false," then after rebooting the ESP32 all options would be false.

Only those two settings are not respecting user choice. I believe the most likely culprit is somewhere in the routine that saves to the EEPROM as those values are only read on boot, whereas runtime is done on the fly and runtime is working but power cycling is not. I do not believe that the changes you made just now are related to the issue (as evidenced by the change merely reversing the effect).

I will write some debug code into the ESP32 on my end and see if I can determine where in the chain there is a break.

[DustinGrau]

It just occurred to me that there might be a similar problem with where we set the values to the preferences that is not overwriting or is using a default somehow. That code would be somewhere in the webhandler file. Unfortunately, I'm bogged down with other projects today and have some travel this afternoon so I've mostly burned the time that I could dedicate to this today.

[nomakewan]

No worries. This might even be what's causing the weird initial wifi issues some people experience after doing the first flash (including me), it occurs to me. I'll continue poking at it to see if I can figure out what's going on.

EDIT: So far I have confirmed that the actual settings page and its associated boolean-to-json parameters are indeed working correctly. I inserted this bit of code into the "Update Settings" functionality:

      if(b_enable_vibration) {
        if(jsonBody["vibration"].as<boolean>()) {
          jsonBody["status"] = "b_enable_vibration is set to true, json also true.";
        }
        else {
          jsonBody["status"] = "b_enable_vibration is set to true, json is false.";
        }
      }
      else {
        if(jsonBody["vibration"].as<boolean>()) {
          jsonBody["status"] = "b_enable_vibration is set to false, json is true.";
        }
        else {
          jsonBody["status"] = "b_enable_vibration is set to false, json is also false.";
        }
      }

And indeed received the popup message, "b_enable_vibration is set to false, json is also false." So the basic functionality sanity checks properly.

[DustinGrau]

Given that you noted the behavior was "saved" and kept at runtime that seemed to mean the logic is working in the JS and backend logic. It's the saving to the preferences that appears to be the issue as you're not getting the right values when the device boots up.

[nomakewan]

We have an interesting result now. I inserted some new code into the complete message, removing the above:

      jsonBody.clear();
      preferences.begin("device", true); // Access namespace in read-only mode.
      uint8_t temp = preferences.getUChar("vibration_enabled", 42);
      preferences.end();
      if(temp == 0){
        jsonBody["status"] = "vibration_enabled is set to 0.";
      }
      else if(temp == 1){
        jsonBody["status"] = "vibration_enabled is set to 1.";
      }
      else if(temp == 42){
        jsonBody["status"] = "vibration_enabled is defaulting to 42.";
      }
      else{
        jsonBody["status"] = "Good people do not end up here.";
      }
      serializeJson(jsonBody, result); // Serialize to string.
      request->send(200, "application/json", result);

By reading the vibration_enabled parameter in Preferences using getUChar instead of getBool I can allow for a non-boolean default value (and in fact getBool uses getUChar internally). And indeed, when I save settings I get this message:

To sanity check my debug function, I then changed vibration_enabled to buzzer_enabled and re-ran the test. The result:

Houston, we have a problem.

EDIT: For reference, the code for getUChar is here: https://github.com/espressif/arduino-esp32/blob/master/libraries/Preferences/src/Preferences.cpp#L368

[DustinGrau]

You confirmed the JSON is valid and comes back as expected. It checks that the value is an unsigned int and obtains the value using .as() which should be a true/false. Though perhaps we should check if the value is truly 1 and otherwise set an explicit FALSE for the variable? Maybe there's bad or untyped data getting put into the bool variable, which is really just a byte so it could theoretically take any value that fits. Just like when we deal with negative values that may be assigned but changed to unexpected values due to the type of the var.

I have no idea where the 42 comes from, unless it's some kind of easter egg for an unknown value.

[nomakewan]

I have no idea where the 42 comes from, unless it's some kind of easter egg for an unknown value.

I added that myself by using getUChar with a default value of 42, then checked against it in the code and wrote a string that told me it was returning the default value of 42. This default value for getUChar can only be returned in two scenarios. First, that _started is false, meaning the Preferences was not initialized with a begin() statement. Since that function returns a bool I added a check just to be extra special sure, and yes, it is indeed starting correctly.

That leaves the only possibility as being an invalid key name. For whatever reason, it is not able to locate the key named vibration_enabled within the Preferences NVS.

[DustinGrau]

FUUUUUUCKKKK!!! The max key size is 15 characters.

[nomakewan]

WELP. Perhaps there are a great many keys which need to be checked now.

[DustinGrau]

Change these just for consistency: use_vibration use_buzzer use_overheat

[DustinGrau]

I recall now there was a limitation, but I kept thinking it was 32 or something divisible by 8. The fact that it's 15 means that it must still adhere to that idea (happens to be 1 less than 16) but perhaps automatically includes a delimiter or separator after the key name internally. In any case the other names don't run afoul but those items you mentioned happened to be 17 chars for the key name.

[nomakewan]

Just tested that change and can confirm that these values now obey user settings.

I'll have to go through and check to make sure there aren't any other Preferences keys that are naughty.

EDIT: No more naughty keys found. Will go ahead and commit the changes and get this issue closed!