-
Notifications
You must be signed in to change notification settings - Fork 272
Several options ( Layout, Notch, Bandpass Filter, widgets,etc. ) in DefaultSettings.json are not applied by the app. #969
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
Comments
I will review this carefully, and I have assigned this issue to myself. Thank you for bringing this up! |
Hello Richard,
thanks to take this in account.
As i'm new in the contribution process with fork and pull request, i'm not
sure to have pull the right last one (may be forget a prior commit of my
branch fixSettings)
But, in any case, the pirla_README.md has all the (tiny) code to apply.
I had leaved a question inside about galea and streaming code that are
excluded of initial settings. But probably, we must load stored settings
for them too.
Ready to exchange if this help.
have a nice day
Pirla , from France
Le mar. 4 mai 2021 à 18:36, Richard Waltman ***@***.***> a
écrit :
… App must take care of the current defaultSettings.json values at startup.
I have proposed a pull request #967
<#967> with all explanations (
pirela_README.md) and solution (quite simple since settings refactoring :) )
Was closed probably because no relation with a bug. Now, there is one.
pirla_README.md
I will review this carefully, and I have assigned this issue to myself.
Thank you for bringing this up!
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#969 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABADNMK7Y6LA37H3HP4J4ILTMAPBFANCNFSM44BHI5KQ>
.
|
@pirelaurent I will see if this can be an easy fix, but I proposed a much broader and general fix to these kinds of issues, found here: |
Hello,
I saw #970 to generalize saving of widgets configuration, but my #969 is
not on the same level, it's on the dynamic of settings, not on the content.
(Just want that defaults settings previously saved are taken in account if
it exists at startup before rewriting a fresh one. Same will stay if 970
was done)
thanks
Le mer. 5 mai 2021 à 23:29, Richard Waltman ***@***.***> a
écrit :
… @pirelaurent <https://github.com/pirelaurent> I will see if this can be
an easy fix, but I proposed a much broader and general fix to these kinds
of issues, found here:
#970 <#970>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#969 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABADNMPQH24HP5EBDHSKX63TMG2FLANCNFSM44BHI5KQ>
.
|
DefaultSettings.json is not meant to be used like this. It's meant to be the default configuration the GUI starts with. We overwrite this every time to account for newer versions of the GUI. You will want to save and load from User settings files that are made with pressing From the GUI console log:
|
This message should be changed. The GUI will no longer auto-load saved user settings. This was a decision by the internal team some time ago. @pirelaurent The quickest way to load your previous settings is to:
|
@pirelaurent Session settings will no longer be auto-saved when users exit a session. This could have been overriding what a user intends to save, but then may make other changes afterwards before closing a session. Starting with GUI 5.0.5, session settings will only be saved or loaded when a user explicitly decides to do so with TopNav buttons or Keyboard presses ( |
@richard
ok, this is your choice but you don't take care of the 3 aspects of a
professional app: user settings, default settings and factory settings.
To day with an external default config written by app at each startup you
have factory settings, not really default one as you can't change it for
next startup (goal of my patch, asked by a user) .
Allowing to designate a default config is mandatory to work (especially in
a team) without source of errors.
More, it is on the trajectory of an automatic startup in case of power
fail.
Please, think about re-establishing real default settings in your revision.
Le sam. 29 mai 2021 à 02:05, Richard Waltman ***@***.***> a
écrit :
… @pirelaurent <https://github.com/pirelaurent> Session settings will no
longer be auto-saved when users exit a session. This could have been
overriding what a user intends to save, but then may make other changes
afterwards before closing a session.
Starting with GUI 5.0.5, session settings will only be saved or loaded
when a user explicitly decides to do so with TopNav buttons or Keyboard
presses (n and N).
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#969 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABADNMMNI33X72GHAPGHG6TTQAVWHANCNFSM44BHI5KQ>
.
|
@pirelaurent This software is released under MIT license and you are free to modify the app to suite your needs. I also have to consider the needs of the international community and our internal team. This is what is best for now. Take Care, |
Problem
DefaultSettings file is overwritten by app at every startup. Whatever you have saved as the default config, it is lost.
A main drawback motivation is for example Notch. If you leave in a country at 50hz, you must change by hand the notch each time even if you saved previously on defaultsettings (or you patch the file ) .
Expected
App must take care of the current defaultSettings.json values at startup.
I have proposed a pull request #967 with all explanations ( pirela_README.md) and solution (quite simple since settings refactoring :) )
Was closed probably because no relation with a bug. Now, there is one.
pirla_README.md
Operating System and Version
WIndows
GUI Version
5.0.4
Running standalone app
visual studio code or standalone
Type of OpenBCI Board
Cyton
Are you using a WiFi Shield?
No
Console Log
Paste any relevant text from the console window here
The text was updated successfully, but these errors were encountered: