jomjol / AI-on-the-edge-device

Easy to use device for connecting "old" measuring units (water, power, gas, ...) to the digital world
https://jomjol.github.io/AI-on-the-edge-device-docs/
5.99k stars 648 forks source link

16.0.0-RC1: Zoom enabled is discarded after upgrade #3277

Closed nliaudat closed 1 month ago

nliaudat commented 1 month ago

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 commented 1 month ago

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

caco3 commented 1 month ago

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

nliaudat commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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

nliaudat commented 1 month ago

the reference image and camera settings do no match the config

caco3 commented 1 month ago

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 commented 1 month ago

@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 commented 1 month ago

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 commented 1 month ago

@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 commented 1 month ago

@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 commented 1 month ago

@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 commented 1 month ago

According to the diff those are the changes: image

caco3 commented 1 month ago

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 commented 1 month ago

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

SybexX commented 1 month ago

@p0macs try Strg + F5