-
Notifications
You must be signed in to change notification settings - Fork 3
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
Qsettings #519
base: master
Are you sure you want to change the base?
Qsettings #519
Conversation
Suggested wording for continuedIncidentWindowDays: on startup, look for any radiolog sessions that were run this many or fewer days ago, and allow the user to start a new operational period of that same incident, incrementing the OP number and the next clue number from those in the previous session chunky for sure - feel free to reword! |
|
||
That should do it! Just run 'python radiolog.py' to run the program. You may want to create a desktop shortcut and use the included icon. | ||
You can optionally provide a logo image to be included on RadioLog printouts | ||
(except for clue reports). The file will be scaled to print in the pdf files, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove '(except for clue reports)' - that applied to clue reports when pdftk was used, but now that reportlog is use for clue reports, the logo will show up there just like the other generated pdfs.
file should be located at: | ||
<Drive>:\Users\<Current User>\AppData\Roaming\NCSSAR\RadioLog\ | ||
Name the file radiolog_logo.jpg. Note that a default logo is included in local_default | ||
so will be copied in to place the first time you run RadioLog. You can then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line (my old wording) is vague - could you suggest a rewording? Maybe just delete the last sentence 'you can then...'
delete radiolog_logo.jpg or overwrite it with your own logo. | ||
|
||
|
||
That should do it! Just run 'python radiolog.py' to run the program. You may |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'run python radiolog.py' only applies when installed from source - not sure if that's clear in the rendered .md? Even if it's under the install from source header, it might be good to spell out that it doesn't apply to installed-from-binary. Either here or above, it would also be good to spell out that the binary installer gives you an executable to run, instead of running python from the command line. Your choice on wording.
@@ -302,6 +302,25 @@ | |||
# ############################################################################# | |||
# ############################################################################# | |||
|
|||
from PyQt5.QtCore import QSettings, QStandardPaths |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the ui imports do need to go below the ui and qrc rebuild code, currently line 569 of master. That was to accommodate #471
@@ -3380,7 +3358,7 @@ def openNewEntry(self,key=None,callsign=None,formattedLocString=None,fleet=None, | |||
else: | |||
self.newEntryWidget.ui.datumFormatLabel.setText("") | |||
|
|||
def newEntry(self,values,amend=False): | |||
def newEntry(self,values): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to preserve amend=False argument (from a recent merge)
@@ -3428,10 +3406,9 @@ def newEntry(self,values,amend=False): | |||
model.endInsertRows() | |||
## if not values[3].startswith("RADIO LOG SOFTWARE:"): | |||
## self.newEntryProcessTeam(niceTeamName,status,values[1],values[3]) | |||
self.newEntryProcessTeam(niceTeamName,status,values[1],values[3],amend) | |||
self.newEntryProcessTeam(niceTeamName,status,values[1],values[3]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to preserve amend argument (from a recent merge)
|
||
def newEntryProcessTeam(self,niceTeamName,status,to_from,msg,amend=False): | ||
# rprint('t1: niceTeamName={} status={} to_from={} msg={} amend={}'.format(niceTeamName,status,to_from,msg,amend)) | ||
def newEntryProcessTeam(self,niceTeamName,status,to_from,msg): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to preserve amend=False argument (from a recent merge)
@@ -3456,34 +3433,8 @@ def newEntryProcessTeam(self,niceTeamName,status,to_from,msg,amend=False): | |||
self.ui.tabWidget.tabBar().tabButton(i,QTabBar.LeftSide).setStyleSheet(statusStyleDict[status]) | |||
# only reset the team timer if it is a 'FROM' message with non-blank message text | |||
# (prevent reset on amend, where to_from can be "AMEND" and msg can be anything) | |||
# if this was an amendment, set team timer based on the team's most recent 'FROM' entry |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to preserve this code - from #411
@@ -3891,7 +3842,7 @@ def tabContextMenu(self,pos): | |||
newEntryFromAction=menu.addAction("New Entry FROM "+str(niceTeamName)) | |||
newEntryToAction=menu.addAction("New Entry TO "+str(niceTeamName)) | |||
menu.addSeparator() | |||
printTeamLogAction=menu.addAction(QIcon(QPixmap(":/radiolog_ui/icons/print_icon.png")),"Print "+str(niceTeamName)+" Log") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to preserve /icons/ in the path - this was fixed in #489
# 406: apply the same fix as #393: | ||
# if the new entry's extTeamName is a case-insensitive match for an | ||
# existing extTeamName, use that already-existing extTeamName instead | ||
for existingExtTeamName in self.extTeamNameList: | ||
if extTeamName.lower()==existingExtTeamName.lower(): | ||
extTeamName=existingExtTeamName | ||
break |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
preserve code from #406
@@ -4425,23 +4369,15 @@ def rebuildTeamHotkeys(self): | |||
l=QLabel(hotkey) | |||
l.setFixedWidth(bar.tabRect(i).width()) | |||
# l.setStyleSheet("border:0px solid black;margin:0px;font-style:italic;font-size:14px;border-image:url(:/radiolog_ui/blank-computer-key.png) 0 0 30 30;") | |||
l.setStyleSheet("border:0px solid black;margin:0px;font-style:italic;font-size:14px;background-image:url(:/radiolog_ui/icons/blank-computer-key.png) 0 0 30 30;") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
preserve /icons/ in path
# #488: make sure standard hotkeys are re-enabled when last team tab is hidden | ||
if bar.count()<2: | ||
self.ui.teamHotkeysWidget.setVisible(False) | ||
|
||
def toggleTeamHotkeys(self): | ||
# #488: don't try to toggle team hotkeys when count is 0 or 1 | ||
# (the dummy tab will still exist after the only team tab has been hidden) | ||
# (should also disable team hotkeys when all team tabs have been hidden) | ||
if self.ui.tabWidget.tabBar().count()<2: | ||
return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
preserve code from #488
if count<4: # hide the window (don't just lower - #504) if the stack is empty | ||
# rprint("lowering: count="+str(count)) | ||
if count<4: # lower the window if the stack is empty | ||
# rprint("lowering: count="+str(count)) | ||
self.setWindowFlags(self.windowFlags() & ~Qt.WindowStaysOnTopHint) # disable always on top | ||
self.hide() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
preserve code from #504
# note the pause happens in newEntryWidget.updateTimer() | ||
if tab.ui.messageField.text()=="" and tab.lastModAge>60: | ||
rprint(' closing unused new entry widget for '+str(tab.ui.teamField.text())+' due to inactivity') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
preserve code from #504 - it would be a valuable part of the transcript in the future, and doesn't happen frequently enough to worry about file size
@@ -4935,7 +4868,7 @@ def __init__(self,parent,sec=0,formattedLocString='',fleet='',dev='',origLocStri | |||
if amendFlag: | |||
self.ui.timeField.setText(row[0]) | |||
self.ui.teamField.setText(row[2]) | |||
if row[1]=="TO": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
preserve this bug fix from #502
# #502 - if amending, call setStatusFromTeam - otherwise it is only called by typing in the callsign field | ||
if self.amendFlag: | ||
self.setStatusFromTeam() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
preserve code from #502
# use to_from value "AMEND" and blank msg text to make sure team timer does not reset | ||
self.parent.newEntryProcessTeam(niceTeamName,status,"AMEND","",self.amendFlag) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
preserve amend argument
|
||
# reapply the filter on team tables, in case callsign was changed | ||
for t in self.parent.ui.tableViewList[1:]: | ||
t.model().invalidateFilter() | ||
else: | ||
self.parent.newEntry(self.getValues(),self.amendFlag) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
preserve amend argument
@@ -5235,7 +5165,7 @@ def accept(self): | |||
v=val[:] # v is a fresh, independent copy of val for each iteration | |||
v[2]=getNiceTeamName(attachedCallsign) | |||
v[3]="[ATTACHED FROM "+self.ui.teamField.text().strip()+"] "+val[3] | |||
self.parent.newEntry(v,self.amendFlag) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
preserve amend argument
@@ -6577,7 +6506,7 @@ def __init__(self,datain,parent=None,*args): | |||
QAbstractTableModel.__init__(self,parent,*args) | |||
self.arraydata=datain | |||
self.printIconPixmap=QPixmap(20,20) | |||
self.printIconPixmap.load(":/radiolog_ui/icons/print_icon.png") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
preserve /icons/ in path
self.filteredIcon=QIcon(QPixmap(":/radiolog_ui/icons/fs_redcircleslash.png")) | ||
self.unfilteredIcon=QIcon(QPixmap(":/radiolog_ui/icons/fs_greencheckbox.png")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
preserve /icons/ in path
@@ -6732,7 +6661,7 @@ def filterAcceptsRow(self,row,parent): | |||
addAllFlag=True | |||
### return(val==target or self.sourceModel().index(row,6,parent).data()==1e10) | |||
## return(val==target) # simple case: match the team name exactly | |||
return(val.lower()==target.lower() or addAllFlag) # #453: perform case-insensitive match |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
preserve code from #453
I added several comments on radiolog.py about chunks of code from recent fixes that didn't make it into this PR. |
A thought on backward compatibility: is there a good way to import settings from the local dir of a previous install, other than copying the file by hand? Maybe part of the installer? Also, would it be easy to place the .ini in a path that's not as far down the hierarchy? Maybe this interacts with the larger issue of where to store files? Maybe something like
If it makes more sense to save that topic for later, that's fine - though forward- and backward- compatibility of .ini file locations would definitely help. |
So it is clear I'll need to rebase on top of the master branch to make this clean and include all changes. I'll take care of that tomorrow. Yes there can be transition code, we had talked about this a bit and it sounded like a manual move was acceptable, but it can be done in the code too if needed. As to the location of the config files, well, you really really don't want them in the source directory. If you do that, every time an installer runs, it will blow away your configuration. Those locations are not my personal locations of choice they are the recommended locations for an applications state and configuration: https://www.thewindowsclub.com/local-localnow-roaming-folders-windows-10#:~:text=What%20is%20Roaming%20folder%20in,its%20favorites,%20documents,%20etc. So that location is chosen by QT/MS as the right location to store things like settings (well MS would prefer the registery, but we've already chatted about that). The radiolog session files should ultimately NOT be in the appdata folder, I expect rather somewhere in Documents or some such. radiolog_rc... probably belongs in appdata as does the fleetsync CSV you are using. But it is programming and you can of course do as you like. Those are just the strong recommendations from MS and increasingly followed by a lot of apps. Let me know what you would like there. |
I have a hard time remembering everything we went over - thanks for the reminder. Thinking about it fresh: 'settings migration code', from before this PR (radiolog.cfg) to after this PR (RadioLog.ini) probably would be good, but a) let's not delay this PR or the subsequent release - for now, migration by hand is probably OK - and b) is it better to have a separate settings migration script script run by the installer as needed, or, as part of the radiolog code? In that last comment, I might have just been thinking of preserving settings (and the print icon) rather than clobbering them, when radiolog is upgraded in the future, where both the old and new versions use RadioLog.ini. Preservation of the settings is probably a given, but just wanted to bring it up. That's kind of what the local_default directory was meant to do originally: those files get copied into place if there were no files in local at runtime, but otherwise it leaves the local dir as-is. Config file location: thanks for clarifying, let's also not delay the PR and release for this, so let's keep them in the recommended location for now. In the future, maybe the installer could be made to do the install-default-config-and-logo-if-they-don't-already-exist task? |
If I can share a user perspective, Documents\RadioLog is a nice spot for
session files and my druthers would be to keep as much as possible out of
appdata\.......
I much prefer keeping configuration stuff in the local folder under
RadioLog. The logo, .ini or config files, even fleetsync.csv. It's easier
to find if you need to make a configuration change, and I would think it
might make cross platform development to simpler, should you decide to port
it to Linux or another OS in the future.
That's my 50kb worth (or however big this message is).
John
…On Sun, Oct 2, 2022, 6:05 PM Tom Grundy ***@***.***> wrote:
I have a hard time remembering everything we went over - thanks for the
reminder. Thinking about it fresh:
'settings migration code', from before this PR (radiolog.cfg) to after
this PR (RadioLog.ini) probably would be good, but a) let's not delay this
PR or the subsequent release - for now, migration by hand is probably OK -
and b) is it better to have a separate settings migration script script run
by the installer as needed, or, as part of the radiolog code?
In that last comment, I might have just been thinking of preserving
settings (and the print icon) rather than clobbering them, when radiolog is
upgraded in the future, where both the old and new versions use
RadioLog.ini. Preservation of the settings is probably a given, but just
wanted to bring it up. That's kind of what the local_default directory was
meant to do originally: those files get copied into place if there were no
files in local at runtime, but otherwise it leaves the local dir as-is.
Config file location: thanks for clarifying, let's also not delay the PR
and release for this, so let's keep them in the recommended location for
now. In the future, maybe the installer could be made to do the
install-default-config-and-logo-if-they-don't-already-exist task?
—
Reply to this email directly, view it on GitHub
<#519 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AM6GMSQCHQFK7ZUGEFOZ6OTWBIWLVANCNFSM6AAAAAAQ2TFG7Y>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Seems like a roadmap of sorts might help. I'd like to get several of the recent changes installed on our systems as soon as possible, and while that could be done any time from source, I'd like to do so as a release so we have an executable and a version number to point at. (Thanks Erin for making that happen, I think the versioning and bumpver and the installer was a HUGH milestone.) I agree that centralizing all the radiolog goop, other than the executable of course, into one dir (C:\Documents\RadioLog makes sense) would be nicest and most user friendly, especially for non-techies who don't want to / don't have the skill to dig in appdata, but I think that can wait a bit. How bout this idea: 3.0.1 - the current release (I'd like to do one or two more 'helper' commits after this PR before 3.1.0: show the version number in the main window banner, and, add some type of feature to view the transcript. If there are other similar minor helpers that anyone feels strongly should be in place before the next release, please chime in.) Thoughts? |
Open to suggestions on that roadmap, by the way. Maybe it makes more sense to slow down and tackle the directory structure question now? If that's the case, settings plus directory structure plus a few more enhancements definitely qualifies as 4.0.0 in my mind. |
So my thoughts here and again as usual this is your program to do as you like with. The location of the config file is 'correct' per MS. So far it has been stated on the one hand that only advanced users should be able to change preferences, hence no GUI for it, but on the other it is too advanced to have it nested in that location. So I'd say decide what you like there. In terms of cross platform simplicity, well this is about as simple as it comes. For each platform the 'right' location is chosen by QT and the settings are stored there. There is no additional code needed for this at this point to handle MacOS nor Linux, it just works. It really doesn't get much cleaner. Keeping config files in c:\radiolog is basically a no go with an installer (setting aside that the installer installs into Program Files) because it will simply be blown away on every upgrade. I suppose work could be done to mark certain files as config files and then upgrade everything around those, but realistically you are working hard to go against years of accumulated knowledge saying to put preferences (mutable data) into appdata (on windows, well really the registry, but appdata if not registry). Could it go elsewhere? Sure. It could go into Documents, or Pictures or anywhere else really but a config file isn't really a document or a picture. It is in fact appdata. And most, not all, but most, apps hold their data in appdata. All that being said, anything is possible when coding, you could store the config file embedded in an image if you wanted to, I'm not here to judge, I just try to go with the overall standards so that when a user comes in and knows that appdata on windows generally lives in appdata, and they look for the appdata for radiolog there, well there it is. For a road map, well I'd say set reasonable expectations. If the config location is going to move, better to move it just once, not once and then again in the next release. I again recognize there is a relatively small and savvy set of users of this software, but that in general is a good habit to get into if possible. So let me know what y'all want. It is our busy season for SAR so I have not had a lot of time to poke at this, but I'll try. |
Yeah, good point, better not to change the config file location twice. In that case, now seems like the time to make the dir structure overhaul. I just added comments on the new issue #522. Modified proposed roadmap: I definitely understand how it sounds like I/we've been giving you doublespeak. But I think the user savvy level is a spectrum, rather than black-and-white: the techiest person on many SAR teams, who is the one who may have to edit the settings file, may not know or want to look in appdata, but they would be more likely to know to look in a centralized working dir. All of the merits you explain for the appdata model or registry model do make sense, and I understand it's much more standardized and common, but I don't think that it's the right model for us. Let's go with the similar model as caltopo desktop (CTD): 1) the install dir, 2) the working dir which includes the settings file. So, I >think< this means waiting on this Settings PR until after the directory structure is overhauled in a separate PR, do you agree? Details of the dir structure proposal are in #522. If it looks good-though-unconventional, I can start work on it later today or tomorrow. |
Hi @erinn, 3.2.0 just got released with the dir structure overhaul (#522). The release page has some short release notes. It's been tested pretty heavily, manually with checklists - but there's always a chance of something getting missed. Also I made a couple of tiny commits directly to master after merging that pull request, in order to clean things up. I >think< those are settled out now, but never say never. I plan to install 3.2.0 in our command vehicles next Saturday. Work for #522 did definitely interact with the config stuff a good bit, mainly to allow importing of an old radiolog.cfg file. So, I probably hosed up some of your work towards QSettings. I do plan to tackle several more bugs and enhancements in the near future - maybe even getting to a newer release before next Saturday. Not terribly sure if any of those would interact with your work on QSettings. |
There's also a new issue #543 - just a whiteboard for a generalized roadmap to v4.0. Right now it's nothing but a list of bugs and enhancements that were opened since July or so. |
I believe I am up to date with your upstream master changes as of this moment.
This work moves from the default configparser to qsettings using the INI format to allow for RadioLog to be used in a platform agnostic way in the future. It also uses pathlib for file locations to allow for platform agnostic paths in the places that it touches.
Take a look at local_default\Radiolog.ini, your
continuedIncidentWindowDays
is included in there but I am unsure of a description to use in that location.I have also moved the initial setup code to using a template to hopefully clarify the location of the radiolog_logo.jpg file. So when it first places the INI file into the location, a couple of variables are substituted.
Finally, the one place that I can't really test currently, is the
utils.ImageReader
code. Using pathlib viastr()
should work there, but double check that.I am also a bit worried that since this became a moving target with all of your work, I may have reverted some of it, so a good look at the diff is in order.