Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

*** OLD *** Migrate configuration -> Superseeded by PR2017 #2001

Closed
wants to merge 10 commits into from

Conversation

caco3
Copy link
Collaborator

@caco3 caco3 commented Feb 8, 2023

Replaced by #2017


This PR adds a migration function of some parameters:

  • Rename section MakeImage to TakeImage, rename all variables and files accordingly
  • Rename Intervall to Interval, rename all variables
  • Rename RSSIThreashold to RSSIThreshold, rename all variables
  • For all boolean parameters: If they where commented out -> remove the semicolon and set them to their default value (according to their C++ initialization).

The migration function gets called at startup and modifies the config file as needed. If one or mor eparameter needs a migration, store the config.ini as config.bak before any change to it.

Yet to do

  • Remove the checkboxes for all boolean parameters in the UI -> @jomjol can you do this? I tried it but failed to understand how it exactly works.
  • @jomjol @haverland @Slider0007: Are there other parameters where you think we should rename them? This PR might be a good time to do so.
  • Testing

@caco3 caco3 marked this pull request as draft February 8, 2023 22:20
@jomjol
Copy link
Owner

jomjol commented Feb 9, 2023

@jomjol @haverland @Slider0007: Are there other parameters where you think we should rename them? This PR might be a good time to do so.

I started to do so, but as the categories are renamed as well, I could not fully test. Will do this in the next days.

@caco3
Copy link
Collaborator Author

caco3 commented Feb 9, 2023

@jomjol You should start with the sd-card/html/edit_config_param.html from this branch, I updated the section/parameters already

@caco3
Copy link
Collaborator Author

caco3 commented Feb 9, 2023

@jomjol There was a merge conflict, so I solved it, see d1d2d67

Not sure if I now reverted some of your work!

@jomjol
Copy link
Owner

jomjol commented Feb 9, 2023

@jomjol There was a merge conflict, so I solved it, see d1d2d67

Not sure if I now reverted some of your work!

I think I just corrected it.

@Slider0007
Copy link
Collaborator

@caco3: Some discussion topics:

config.ini:
AutoAdjustSummertime --> AFAIK this not used anymore
LogfileRetentionInDays --> maybe LogRetentionPeriod (specially for logging the images, Logfile... is kind of misleading, but renaming this is not a must)
SetRetainFlag --> RetainFlag or RetainMessages(renaming not a must)

@caco3
Copy link
Collaborator Author

caco3 commented Feb 11, 2023

config.ini: AutoAdjustSummertime

Stimmt, das wird in der Firmware nirgends mehr verwendet. @jomjol, können wir das löschen?

LogfileRetentionInDays --> maybe LogRetentionPeriod (specially for logging the images, Logfile... is kind of misleading, but renaming this is not a must)

Right.
I would suggest to do it as following:

  • Analog/Digits: Here we use it for the training images. I would rename it to RoiImagesRetentionInDays or TrainingImagesRetentionInDays. And I would rename the related parameters as well. Ans also update the default path (eg. /log/analog)
  • Debug: Keep LogfileRetentionInDays as is.

SetRetainFlag --> RetainFlag or RetainMessages(renaming not a must)

I would go for RetainMessages

@jomjol, what is LogImageLocation in the section TakeImage used for? Is that storning the whole cam image?

@jomjol
Copy link
Owner

jomjol commented Feb 12, 2023

config.ini: AutoAdjustSummertime

Stimmt, das wird in der Firmware nirgends mehr verwendet. @jomjol, können wir das löschen?

Right, we can delete it

LogfileRetentionInDays --> maybe LogRetentionPeriod (specially for logging the images, Logfile... is kind of misleading, but renaming this is not a must)

Right. I would suggest to do it as following:

* Analog/Digits: Here we use it for the training images. I would rename it to `RoiImagesRetentionInDays` or `TrainingImagesRetentionInDays`. And I would rename the related parameters as well. Ans also update the default path (eg. `/log/analog`)

TrainingImagesRetention... could make the people think, the device is training itself. Therefore I would go for "ROIImages..."

@jomjol, what is LogImageLocation in the section TakeImage used for? Is that storning the whole cam image?

This is used for storing the full images (before cutting) in the directory /log/source

@caco3
Copy link
Collaborator Author

caco3 commented Feb 12, 2023

I think we should to a release 14.1 which just the parameter migration.
Release 15 will have a lot of changes which might break stuff. And this way users can go back top v14 without broken parameters.

I will prepare a branch for it.

@caco3 caco3 changed the base branch from rolling to v14.1 February 12, 2023 08:48
@caco3 caco3 changed the base branch from v14.1 to rolling February 12, 2023 09:31
@caco3 caco3 changed the title Migrate configuration *** OLD *** Migrate configuration -> Superseeded by PR2017 Feb 12, 2023
@caco3
Copy link
Collaborator Author

caco3 commented Feb 12, 2023

New PR which only contains the parameter changes: #2017

@caco3 caco3 closed this Feb 12, 2023
@caco3 caco3 mentioned this pull request Feb 12, 2023
@caco3 caco3 deleted the migrate-config branch February 21, 2023 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants