Here are the settings we should consider saving for each module:
List:
- sort order
Map:
- Selected satellite
- Show ground track
- highlight footprint (maybe we should just remove this setting - I don't think anyone ever disables it?)
Polar view:
- Show sky track (would the global "always show" flag override this or the other way around?)
Single sat:
- Selected satellite
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I use the disable highlight footprint when I create modules with a large number of satellites so I can compare views. Actually I was thinking if disabling the outline as well.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Things which apply for list view should also apply for event view.
An aside on this is that if we allow multiple instances of the same module type, then event view and list view could be collapsed into a single view type. The user could then just select the columns wanted and sort order and they are equivalent.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Saving the configuration at line 1175 is not necessary since the destroy function also saves the configuration. You can check the log files - it will contain "configuration saved" twice.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
The changes at line 980 get run every pass through the code and update the los and aos values as soon as they are stale. It does not wait for the event count. This was in my code from some other work I was doing trying to make the list view update. What was happening was the most recent AOS was still showing in the table even though the AOS was in the past. It would seem we want both the event driven for when time is reversed and the stale trigger to make sure we are update. Agreed that this is outside the scope of this patch. I can revert it.
The code in line 1175 is added because when I close using the x graphic in the corner the module was not being saved. I agree that it saves it twice when closing the app, but this seemed better than not saving at all when the module was close other ways.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
That's strange because when I close a module using the X, I still get the log entry twice. The destroy signal should always be executed when a widget is destroyed so maybe there is a bug somewhere.
Concerning the AOS/LOS changes, if there is an issue we should fix it of course, though I don't understand what you mean with AOS in the past - were you using the time controller or what happened? Maybe explain it in the code what stale means and under which conditions it occurs because without understanding it the logic seems quite incomprehensible in my opinion (and please use spacebar between /* and the words ;-)
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I think it might deal with the fact that I do not load the selected satellite into the cfgdata structure until the destroy function is called on the view.
There is also another quirk that the module can be saved after the logging engine shuts down so the message appears on the console.
As for the aos fiasco, I wish I made sure I had a clean check-in.
The logic is as follows, the event_count conditional is the original code. It updates the aos/los at regular intervals. However AOS/LOS will almost certainly occur between these updates. When sat->aos is earlier than daynum, it occured in the past if daynum is the present. In this case the value in sat->aos is not the next aos. It is a past aos. Therefore it should be updated. The same logic applies with LOS. This should be the straightforward part.
sat->aos > 0 is a way of caching the result of the has_aos function from the past. This may be cryptic.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Oh, you mean the delay between AOS and the next timeout. Now I understand.
Well, if that delay is an issue we can switch to event based AOS/LOS calculation and eliminate the timeout based calculations; having both seems redundant. Technically, the code you added is sufficient provided that an initial calculation has been made and time only goes forward.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Having both is needed to handle the superstationary satellites. They have such slow orbits that for some period of time find_aos will return zero because there is not an aos for several weeks. However when we are closer to the aos, find_aos will find the valid aos.
Changing to a completely event driven paradigm would require changing the max_dt paradigm. It would also require tagging los and aos events with information about the qth. The timeout paradigm also nicely handles the tle updates.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
SVN checkin 903 should handle the polar view saving show track information. It stores which satellites have been explicitly hidden or show and honors this before module preference. This leaves the sort order on the two flavors of lists and this feature request can be closed out.
There is some cut and paste code from other files that should ultimately be moved to gpredict-utils or similar location.
As always comments welcome, bug reports welcome.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Regarding the implementations in the map and polar views, the load and store functions should have the same name (except the load/store parts) to indicate that they are used to load and store the same data. The name should reflect the actual data type.
Furthermore, it seems that these are generic functions and repeated in both files, so why not move them to e.g. mod-cfg? Everything else in the map and polar view uses the mod-cfg wrapper API and we should avoid direct access to g_key_file if we can.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Here are the settings we should consider saving for each module:
List:
- sort order
Map:
- Selected satellite
- Show ground track
- highlight footprint (maybe we should just remove this setting - I don't think anyone ever disables it?)
Polar view:
- Show sky track (would the global "always show" flag override this or the other way around?)
Single sat:
- Selected satellite
I use the disable highlight footprint when I create modules with a large number of satellites so I can compare views. Actually I was thinking if disabling the outline as well.
Things which apply for list view should also apply for event view.
An aside on this is that if we allow multiple instances of the same module type, then event view and list view could be collapsed into a single view type. The user could then just select the columns wanted and sort order and they are equivalent.
svn commit 825 saves the selected satellite in single sat view.
Comments welcomed.
I'm confused about the changes in gtk-sat-module.c line 980
http://gpredict.svn.sourceforge.net/viewvc/gpredict/trunk/src/gtk-sat-module.c?r1=818&r2=825&sortby=date
Seems to be unrelated to this and even within another scope, I don't understand the purpose; AOS and LOS were just calculated in the lines above.
Saving the configuration at line 1175 is not necessary since the destroy function also saves the configuration. You can check the log files - it will contain "configuration saved" twice.
The changes at line 980 get run every pass through the code and update the los and aos values as soon as they are stale. It does not wait for the event count. This was in my code from some other work I was doing trying to make the list view update. What was happening was the most recent AOS was still showing in the table even though the AOS was in the past. It would seem we want both the event driven for when time is reversed and the stale trigger to make sure we are update. Agreed that this is outside the scope of this patch. I can revert it.
The code in line 1175 is added because when I close using the x graphic in the corner the module was not being saved. I agree that it saves it twice when closing the app, but this seemed better than not saving at all when the module was close other ways.
That's strange because when I close a module using the X, I still get the log entry twice. The destroy signal should always be executed when a widget is destroyed so maybe there is a bug somewhere.
Concerning the AOS/LOS changes, if there is an issue we should fix it of course, though I don't understand what you mean with AOS in the past - were you using the time controller or what happened? Maybe explain it in the code what stale means and under which conditions it occurs because without understanding it the logic seems quite incomprehensible in my opinion (and please use spacebar between /* and the words ;-)
I think it might deal with the fact that I do not load the selected satellite into the cfgdata structure until the destroy function is called on the view.
There is also another quirk that the module can be saved after the logging engine shuts down so the message appears on the console.
As for the aos fiasco, I wish I made sure I had a clean check-in.
The logic is as follows, the event_count conditional is the original code. It updates the aos/los at regular intervals. However AOS/LOS will almost certainly occur between these updates. When sat->aos is earlier than daynum, it occured in the past if daynum is the present. In this case the value in sat->aos is not the next aos. It is a past aos. Therefore it should be updated. The same logic applies with LOS. This should be the straightforward part.
sat->aos > 0 is a way of caching the result of the has_aos function from the past. This may be cryptic.
Oh, you mean the delay between AOS and the next timeout. Now I understand.
Well, if that delay is an issue we can switch to event based AOS/LOS calculation and eliminate the timeout based calculations; having both seems redundant. Technically, the code you added is sufficient provided that an initial calculation has been made and time only goes forward.
Having both is needed to handle the superstationary satellites. They have such slow orbits that for some period of time find_aos will return zero because there is not an aos for several weeks. However when we are closer to the aos, find_aos will find the valid aos.
Changing to a completely event driven paradigm would require changing the max_dt paradigm. It would also require tagging los and aos events with information about the qth. The timeout paradigm also nicely handles the tle updates.
svn 885 should take care of the map module storage items below.
- Show ground track
- highlight footprint
SVN checkin 903 should handle the polar view saving show track information. It stores which satellites have been explicitly hidden or show and honors this before module preference. This leaves the sort order on the two flavors of lists and this feature request can be closed out.
There is some cut and paste code from other files that should ultimately be moved to gpredict-utils or similar location.
As always comments welcome, bug reports welcome.
As of svn checkin 910 all the list sort order are saved and restored.
I believe this is the last of the todo's under this feature.
I will leave this open but marked fixed.
Regarding the implementations in the map and polar views, the load and store functions should have the same name (except the load/store parts) to indicate that they are used to load and store the same data. The name should reflect the actual data type.
Furthermore, it seems that these are generic functions and repeated in both files, so why not move them to e.g. mod-cfg? Everything else in the map and polar view uses the mod-cfg wrapper API and we should avoid direct access to g_key_file if we can.