On 29/05/2015 13:50, Joe Taylor wrote: > Bill -- Hi Joe, > > Thanks for the quick fix. Probably I should have been able to figure > out what was wrong, myself; but I'm not particularly comfortable with > the use of QVariants, and I hadn't quite figured out what you were doing > with the "frequencies" list. It's not so much the QVariant type but the reason why it is there at all. The Qt Model/View/Controller design principles are what is important here. The application data models are adapted to implement the QAbstractItemModel interface, usually by inheriting one of the helper types like QAbstractListModel or QAbstractTableModel. This allows user types to seamlessly plug into the Qt GUI views. The QVariant type is just an implementation detail that allows Qt and custom types to be used uniformly in the Qt meta-type system.
In this case the frequency list is exposed as QSortFilterProxyModel which wraps a QAbstractListModel derivation of the underlying list of frequencies and their related modes. > > Surely the line > > on_bandComboBox_activated(i); > > should now read > > on_bandComboBox_activated(row); > > ?? Yes, good catch. I am guilty of assuming the compiler would pick that up but that doesn't work with Fortran programmers around with their customary short variable names :( I have fixed it. > > Now might be a good time to mention a growing concern about our code in > WSJT-X. Me too! > > It seems to me that mainwindow.cpp is getting far too big and unwieldy > -- more than 4000 lines now. Too many things have been just "added on", > most recently all the WSPR mode stuff. Arguably the state-machine logic > used in guiUpdate() is complicated, hard to follow, and perhaps > out-of-place anyway in an event-driven program. Yes definitely. I have been picking away at various areas, mainly from the bottom up but progress is slower than the growth at the moment. Also often a re-factoring to fit better to the C++ or Qt idiom can add lines until the surrounding code is also re-factored. > > In short, I think mainwindow.cpp could use a big dose of what I guess is > now called "refactoring" by real programmers (which I'm not). :-) Re-factoring is a definite requirement but it should not be undertaken without unit tests being in place to quickly verify that regressions are avoided. Lack of unit tests are an even bigger deficiency in my opinion than the code bloat in MainWindow. > > Thoughts on this topic from others would be welcome. I still have a local branch in progress that is aimed at unifying jt9 (and probably wsprd as well now) into the main application. I believe this will help as the abstraction of a stream of decodes looks like something that should live in a class hierarchy even if each mode is just a thin wrapper around a Fortran decoding core. Another area that I see as needing attention is the lack of abstraction of the underlying QSO state machine, the diagnosis and debugging of this basic functionality is virtually impossible due to lack of structure. I don't know how good the Qt QStateMachine model is but it looks like a reasonable tool to implement something like WSJT-X, it does link with the Qt property system and signals/slots to interact with the rest of the program logic and the GUI. The issue with adoption would probably lie in making the rest of the code more Qt idomatic by abstracting the relevant parts in the Qt way. > > -- Joe > > On 5/29/2015 6:23 AM, Bill Somerville wrote: >> On 29/05/2015 01:55, Joe Taylor wrote: >>> Hi Bill, >> Hi Joe, >>> Thanks for your work on the frequency/mode combinations. >>> >>> An unwanted side effect is that WSPR-mode band hopping no longer works. >>> Not sure why, but I've determined that the structure >>> >>> auto f = frequency.value<Frequency>(); >>> >>> presently at line 4349 in mainwindow.cpp always evaluates to f=0. >> Sorry about that. The problem is that I had used the Qt compatible data >> model to access the underlying filtered frequency list and the default >> role for the data() access function is the display role which is a >> string including the band name rather than the raw Frequency type value. >> >> I have put in a quick fix for now but I will improve the whole access to >> underlying model data implementation when I get back later. >>> -- Joe >> 73 >> Bill >> G4WJS. >>> On 5/28/2015 7:32 PM, Bill Somerville wrote: >>>> Hi All, >>>> >>>> I have added a change to make working frequencies mode dependent. The >>>> first thing you will notice is that your working frequencies will have >>>> disappeared, this is because the format of the frequencies table has >>>> changed. To restore the frequencies go to Settings->Frequencies and >>>> click the new "Reset" button, this will reset the working frequencies to >>>> default values. >>>> >>>> The default frequencies are defined in FrequencyList.cpp, they are a >>>> work in progress and will almost certainly need adjusting before >>>> release. Ferquencies can either have a specific mode value from the >>>> FrequencyList::Modes enumeration or the NULL_MODE value. If a frequency >>>> is designated as NULL_MODE it will appear whatever mode is chosen. >>>> >>>> I have removed the +2 kHz check box since changing to JT9 will select >>>> the JT9 frequencies and QSY to the one on the current band if there is >>>> one. Going to another mode will QSY to that mode's working frequency if >>>> there is one for the current band. >>>> >>>> This is all WIP so expect a few glitches but I thought that the >>>> functionality improvements my be worth a little pain with defects from >>>> early exposure. As always comments, suggestions and testing is welcome. >>>> >>>> 73 >>>> Bill >>>> G4WJS. ------------------------------------------------------------------------------ _______________________________________________ wsjt-devel mailing list wsjt-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/wsjt-devel
