16.0.0-RC1: Zoom enabled is discarded after upgrade

[nliaudat]

The Problem

The old parameters as zoom = true are not preserved upon update

Version

Release: v16-0-0-RC1 (Commit: ca01f5a+)

Logfile

Old :
____________________

[TakeImage]
;RawImagesLocation = /log/source
WaitBeforeTakingPicture = 4
;RawImagesRetention = 15
Demo = false
Brightness = 0
Contrast = 0
Saturation = 0
Sharpness = 0 
LEDIntensity = 3
ImageQuality = 12
ImageSize = VGA
Zoom = true
ZoomMode = 0
ZoomOffsetX = 700
ZoomOffsetY = 285
Grayscale = false
Negative = false
Aec2 = true
AutoExposureLevel = -1
FixedExposure = false

NEW : 
_______________

[TakeImage]
;RawImagesLocation = /log/source
						   
;RawImagesRetention = 15
WaitBeforeTakingPicture = 4
CamGainceiling = x8
CamQuality = 10
CamBrightness = 0
CamContrast = 0
CamSaturation = 0
CamSharpness = 0
CamAutoSharpness = false
CamSpecialEffect = no_effect
CamWbMode = auto
CamAwb = true
CamAwbGain = true
CamAec = true
CamAec2 = true
CamAeLevel = 2
CamAecValue = 600
CamAgc = true
CamAgcGain = 8
CamBpc = true
CamWpc = true
CamRawGma = true
CamLenc = true
CamHmirror = false
CamVflip = false
CamDcw = true
CamDenoise = 0
CamZoom = true
CamZoomOffsetX = 0
CamZoomOffsetY = 0
CamZoomSize = 0
LEDIntensity = 3
Demo = false

Expected Behavior

No response

Screenshots

Additional Context

[nliaudat]

For zoom, the fix is to set the "zoom size" (29 or more) if CamZoom = true

[caco3]

Yes, it is known that the reference image/camera settings might need an update.
I just added a note to the release for this:

[nliaudat]

Ok, but update the notice to

"The reference image NEEDS to be updated".

Mine was not to dark at all. It was not zoomed anymore

[caco3]

I changed it now to

If the image is too dark after the update or otherwise different than before, you might need to update the camera settings and create a new reference image. We extended the camera settings and some devices need reconfiguration due to this.

[nliaudat]

But the old parameters can be upgraded to new.
That is the problem

[nliaudat]

the reference image and camera settings do no match the config

[caco3]

But the old parameters can be upgraded to new.
That is the problem

What do you suggest?

the reference image and camera settings do no match the config

And here?

[SybexX]

@caco3 To solve the problem you would have to adjust the "void migrateConfiguration(void){}" in the main.cpp

maybe like that:

void migrateConfiguration(void)
{
    bool migrated = false;

    if (!FileExists(CONFIG_FILE))
    {
        LogFile.WriteToFile(ESP_LOG_ERROR, TAG, "Config file seems to be missing!");
        return;
    }

    std::string section = "";
    std::ifstream ifs(CONFIG_FILE);
    std::string content((std::istreambuf_iterator<char>(ifs)), (std::istreambuf_iterator<char>()));

    /* Split config file it array of lines */
    std::vector<std::string> configLines = splitString(content);

    /* Process each line */
    for (int i = 0; i < configLines.size(); i++)
    {
        // ESP_LOGI(TAG, "Line %d: %s", i, configLines[i].c_str());

        if (configLines[i].find("[") != std::string::npos)
        {
            // Start of new section
            section = configLines[i];
            replaceString(section, ";", "", false); // Remove possible semicolon (just for the string comparison)
            // ESP_LOGI(TAG, "New section: %s", section.c_str());
        }

        /* Migrate parameters as needed
         * For the boolean parameters, we make them enabled all the time now:
         *  1. If they where disabled, set them to their default value
         *  2. Enable them
         * Notes:
         * The migration has some simplifications:
         *  - Case Sensitiveness must be like in the initial config.ini
         *  - No Whitespace after a semicollon
         *  - Only one whitespace before/after the equal sign
         */
        if (section == "[MakeImage]")
        {
            migrated = migrated | replaceString(configLines[i], "[MakeImage]", "[TakeImage]"); // Rename the section itself
        }

        if (section == "[MakeImage]" || section == "[TakeImage]")
        {
            if ((isInString(configLines[i], "Brightness")) && (!isInString(configLines[i], "CamBrightness")))
            {
                migrated = migrated | replaceString(configLines[i], "Brightness", "CamBrightness");
            }
            else if ((isInString(configLines[i], "Contrast")) && (!isInString(configLines[i], "CamContrast")))
            {
                migrated = migrated | replaceString(configLines[i], "Contrast", "CamContrast");
            }
            else if ((isInString(configLines[i], "Saturation")) && (!isInString(configLines[i], "CamSaturation")))
            {
                migrated = migrated | replaceString(configLines[i], "Saturation", "CamSaturation");
            }
            else if ((isInString(configLines[i], "Sharpness")) && (!isInString(configLines[i], "CamSharpness")) && (!isInString(configLines[i], "CamAutoSharpness")))
            {
                migrated = migrated | replaceString(configLines[i], "Sharpness", "CamSharpness");
            }
            else if ((isInString(configLines[i], "Aec2")) && (!isInString(configLines[i], "CamAec")) && (!isInString(configLines[i], "CamAec2")))
            {
                migrated = migrated | replaceString(configLines[i], "Aec2", "CamAec2");
            }
            else if ((isInString(configLines[i], "Zoom")) && (!isInString(configLines[i], "CamZoom")))
            {
                migrated = migrated | replaceString(configLines[i], "Zoom", "CamZoom");
            }
            else
            {
                migrated = migrated | replaceString(configLines[i], "LogImageLocation", "RawImagesLocation");
                migrated = migrated | replaceString(configLines[i], "LogfileRetentionInDays", "RawImagesRetention");

                migrated = migrated | replaceString(configLines[i], ";Demo = true", ";Demo = false"); // Set it to its default value
                migrated = migrated | replaceString(configLines[i], ";Demo", "Demo");                 // Enable it

                migrated = migrated | replaceString(configLines[i], "ImageQuality", "CamQuality");
                migrated = migrated | replaceString(configLines[i], "AutoExposureLevel", "CamAeLevel");
                migrated = migrated | replaceString(configLines[i], "FixedExposure", "CamAec");

                // migrated = migrated | replaceString(configLines[i], "Zoom", "CamZoom");
                migrated = migrated | replaceString(configLines[i], "ZoomMode", "CamZoomSize");
                migrated = migrated | replaceString(configLines[i], "CamZoomMode", "CamZoomSize");
                // migrated = migrated | replaceString(configLines[i], "ZoomOffsetX", "CamZoomOffsetX");
                // migrated = migrated | replaceString(configLines[i], "ZoomOffsetY", "CamZoomOffsetY");

                migrated = migrated | replaceString(configLines[i], "ImageSize", ";UNUSED_PARAMETER"); // This parameter is no longer used
                migrated = migrated | replaceString(configLines[i], "Grayscale", ";UNUSED_PARAMETER"); // This parameter is no longer used
                migrated = migrated | replaceString(configLines[i], "Negative", ";UNUSED_PARAMETER");  // This parameter is no longer used
            }
        }
        else if (section == "[Alignment]")
        {
            migrated = migrated | replaceString(configLines[i], "InitialMirror", ";UNUSED_PARAMETER");  // This parameter is no longer used
            migrated = migrated | replaceString(configLines[i], ";InitialMirror", ";UNUSED_PARAMETER"); // This parameter is no longer used
            migrated = migrated | replaceString(configLines[i], "FlipImageSize", ";UNUSED_PARAMETER");  // This parameter is no longer used
            migrated = migrated | replaceString(configLines[i], ";FlipImageSize", ";UNUSED_PARAMETER"); // This parameter is no longer used
        }
        else if (section == "[Digits]")
        {
            migrated = migrated | replaceString(configLines[i], "LogImageLocation", "ROIImagesLocation");
            migrated = migrated | replaceString(configLines[i], "LogfileRetentionInDays", "ROIImagesRetention");
        }
        else if (section == "[Analog]")
        {
            migrated = migrated | replaceString(configLines[i], "LogImageLocation", "ROIImagesLocation");
            migrated = migrated | replaceString(configLines[i], "LogfileRetentionInDays", "ROIImagesRetention");
            migrated = migrated | replaceString(configLines[i], "ExtendedResolution", ";UNUSED_PARAMETER"); // This parameter is no longer used
        }
        else if (section == "[PostProcessing]")
        {
            /* AllowNegativeRates has a <NUMBER> as prefix! */
            if (isInString(configLines[i], "AllowNegativeRates") && isInString(configLines[i], ";"))
            {                                                                         // It is the parameter "AllowNegativeRates" and it is commented out
                migrated = migrated | replaceString(configLines[i], "true", "false"); // Set it to its default value
                migrated = migrated | replaceString(configLines[i], ";", "");         // Enable it
            }
            /* IgnoreLeadingNaN has a <NUMBER> as prefix! */
            else if (isInString(configLines[i], "IgnoreLeadingNaN") && isInString(configLines[i], ";"))
            {                                                                         // It is the parameter "IgnoreLeadingNaN" and it is commented out
                migrated = migrated | replaceString(configLines[i], "true", "false"); // Set it to its default value
                migrated = migrated | replaceString(configLines[i], ";", "");         // Enable it
            }
            /* ExtendedResolution has a <NUMBER> as prefix! */
            else if (isInString(configLines[i], "ExtendedResolution") && isInString(configLines[i], ";"))
            {                                                                         // It is the parameter "ExtendedResolution" and it is commented out
                migrated = migrated | replaceString(configLines[i], "true", "false"); // Set it to its default value
                migrated = migrated | replaceString(configLines[i], ";", "");         // Enable it
            }
            else
            {
                migrated = migrated | replaceString(configLines[i], ";PreValueUse = true", ";PreValueUse = false"); // Set it to its default value
                migrated = migrated | replaceString(configLines[i], ";PreValueUse", "PreValueUse");                 // Enable it

                migrated = migrated | replaceString(configLines[i], ";ErrorMessage = true", ";ErrorMessage = false"); // Set it to its default value
                migrated = migrated | replaceString(configLines[i], ";ErrorMessage", "ErrorMessage");                 // Enable it

                migrated = migrated | replaceString(configLines[i], ";CheckDigitIncreaseConsistency = true", ";CheckDigitIncreaseConsistency = false"); // Set it to its default value
                migrated = migrated | replaceString(configLines[i], ";CheckDigitIncreaseConsistency", "CheckDigitIncreaseConsistency");                 // Enable it
            }
        }
        else if (section == "[MQTT]")
        {
            migrated = migrated | replaceString(configLines[i], "SetRetainFlag", "RetainMessages");                   // First rename it, enable it with its default value
            migrated = migrated | replaceString(configLines[i], ";RetainMessages = true", ";RetainMessages = false"); // Set it to its default value
            migrated = migrated | replaceString(configLines[i], ";RetainMessages", "RetainMessages");                 // Enable it

            migrated = migrated | replaceString(configLines[i], ";HomeassistantDiscovery = true", ";HomeassistantDiscovery = false"); // Set it to its default value
            migrated = migrated | replaceString(configLines[i], ";HomeassistantDiscovery", "HomeassistantDiscovery");                 // Enable it

            if (configLines[i].rfind("Topic", 0) != std::string::npos) // only if string starts with "Topic" (Was the naming in very old version)
            {
                migrated = migrated | replaceString(configLines[i], "Topic", "MainTopic");
            }
        }
        else if (section == "[InfluxDB]")
        {
            /* Fieldname has a <NUMBER> as prefix! */
            if (isInString(configLines[i], "Fieldname"))
            {
                // It is the parameter "Fieldname"
                migrated = migrated | replaceString(configLines[i], "Fieldname", "Field"); // Rename it to Field
            }
        }
        else if (section == "[InfluxDBv2]")
        {
            /* Fieldname has a <NUMBER> as prefix! */
            if (isInString(configLines[i], "Fieldname"))
            {
                // It is the parameter "Fieldname"
                migrated = migrated | replaceString(configLines[i], "Fieldname", "Field"); // Rename it to Field
            }
            /* Database got renamed to Bucket! */
            else if (isInString(configLines[i], "Database"))
            {
                // It is the parameter "Database"
                migrated = migrated | replaceString(configLines[i], "Database", "Bucket"); // Rename it to Bucket
            }
        }
        else if (section == "[GPIO]")
        {
        }
        else if (section == "[DataLogging]")
        {
            migrated = migrated | replaceString(configLines[i], "DataLogRetentionInDays", "DataFilesRetention");
            /* DataLogActive is true by default! */
            migrated = migrated | replaceString(configLines[i], ";DataLogActive = false", ";DataLogActive = true"); // Set it to its default value
            migrated = migrated | replaceString(configLines[i], ";DataLogActive", "DataLogActive");                 // Enable it
        }
        else if (section == "[AutoTimer]")
        {
            migrated = migrated | replaceString(configLines[i], "Intervall", "Interval");
            migrated = migrated | replaceString(configLines[i], ";AutoStart = true", ";AutoStart = false"); // Set it to its default value
            migrated = migrated | replaceString(configLines[i], ";AutoStart", "AutoStart");                 // Enable it
        }
        else if (section == "[Debug]")
        {
            migrated = migrated | replaceString(configLines[i], "Logfile ", "LogLevel "); // Whitespace needed so it does not match `LogfileRetentionInDays`
            /* LogLevel (resp. LogFile) was originally a boolean, but we switched it to an int
             * For both cases (true/false), we set it to level 2 (WARNING) */
            migrated = migrated | replaceString(configLines[i], "LogLevel = true", "LogLevel = 2");
            migrated = migrated | replaceString(configLines[i], "LogLevel = false", "LogLevel = 2");
            migrated = migrated | replaceString(configLines[i], "LogfileRetentionInDays", "LogfilesRetention");
        }
        else if (section == "[System]")
        {
            migrated = migrated | replaceString(configLines[i], "RSSIThreashold", "RSSIThreshold");
            migrated = migrated | replaceString(configLines[i], "AutoAdjustSummertime", ";UNUSED_PARAMETER"); // This parameter is no longer used

            migrated = migrated | replaceString(configLines[i], ";SetupMode = true", ";SetupMode = false"); // Set it to its default value
            migrated = migrated | replaceString(configLines[i], ";SetupMode", "SetupMode");                 // Enable it
        }
    }

    if (migrated)
    {
        // At least one replacement happened
        if (!RenameFile(CONFIG_FILE, CONFIG_FILE_BACKUP))
        {
            LogFile.WriteToFile(ESP_LOG_ERROR, TAG, "Failed to create backup of Config file!");
            return;
        }

        FILE *pfile = fopen(CONFIG_FILE, "w");

        for (int i = 0; i < configLines.size(); i++)
        {
            if (!isInString(configLines[i], ";UNUSED_PARAMETER"))
            {
                fwrite(configLines[i].c_str(), configLines[i].length(), 1, pfile);
                fwrite("\n", 1, 1, pfile);
            }
        }

        fclose(pfile);
        LogFile.WriteToFile(ESP_LOG_INFO, TAG, "Config file migrated. Saved backup to " + string(CONFIG_FILE_BACKUP));
    }
}

[nliaudat]

In fact, I do not understand why @caco3 is closing bugs on alpha release without investigating.

Actually, the zoom factor on my first cam was very blury and I broke it while turing it left side.
The result is I cannot help you further to debug the problem, but there is one.

Regards and please keep cool with helpers who spend time to find new bugs.

[SybexX]

@nliaudat The zoom function has been completely revised and now works similarly to a cell phone.
It is zoomed to the center and the offset is now +- , CamZoomSize = 0 >> no zoom and CamZoomSize = 29 >> fully zoomed in

[caco3]

@SybexX I was not aware that we renamed the parameters. Then a parameter migration of course makes sense.
Is your proposed code complete or do we have to go through a list and check?

@nliaudat We get a lot of issues and most of them actually should be discussions. Every week I convert a few of them :(
I closed this issue because we know that an update of the parameters is required and we already documented it on the release page. Of course a message directly on the UI would be much nice. But it also would be quite some work for us. We are a small team and also have other hobbies. I already spend far more time for this project than I actually should priority wise.

[SybexX]

@caco3 That should be all of the camera parameters, but I can't say what the others look like.
However, there will still be problems with zoom because there are more zoom levels and the offset is now +-
ZoomMode = 0/1 vs. CamZoomSize = 0 to 29
ZoomOffsetX = 0 to 800 vs. CamZoomOffsetX = +/- 480
ZoomOffsetY = 0 to 600 vs. CamZoomOffsetY = +/- 360

[caco3]

According to the diff those are the changes:

[caco3]

However, there will still be problems with zoom because there are more zoom levels and the offset is now +-
ZoomMode = 0/1 vs. CamZoomSize = 0 to 29
ZoomOffsetX = 0 to 800 vs. CamZoomOffsetX = +/- 480
ZoomOffsetY = 0 to 600 vs. CamZoomOffsetY = +/- 360

That is ok. We can't handle everything automatically.

[p0macs]

The Configuration page refers to the old parameters name, at least for me:

[SybexX]

@p0macs try Strg + F5