<table><tr><td style="">TallFurryMan added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D18235">View Revision</a></tr></table><br /><div><div><p>That's great, agreed. I added a few (as usual) pedantic comments. My setup is out of order while I install the fixed obs, so I don't know when I will be able to test.</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D18235#inline-100661">View Inline</a><span style="color: #4b4d51; font-weight: bold;">capture.cpp:3144</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">m_State</span> <span style="color: #aa2211">==</span> <span class="n">CAPTURE_IDLE</span> <span style="color: #aa2211">||</span> <span class="n">m_State</span> <span style="color: #aa2211">==</span> <span class="n">CAPTURE_ABORTED</span> <span style="color: #aa2211">||</span> <span class="n">m_State</span> <span style="color: #aa2211">==</span> <span class="n">CAPTURE_COMPLETE</span><span class="p">)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">OK so here, Mount is telling Capture it needs to flip, and capture either replies it is OK to flip now, or delays. Correct? So MF_REQUESTED can be set multiple times.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D18235#inline-100662">View Inline</a><span style="color: #4b4d51; font-weight: bold;">capture.cpp:3329</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; "> <span class="p">}</span>
</div><div style="padding: 0 8px; margin: 0 4px; "> <span style="color: #aa4000">else</span> <span style="color: #aa4000">if</span> <span class="p">(</span><span style="color: #aa2211">!</span><span class="n">strcmp</span><span class="p">(</span><span class="n">tagXMLEle</span><span class="p">(</span><span class="n">ep</span><span class="p">),</span> <span style="color: #766510">"MeridianFlip"</span><span class="p">))</span>
</div><div style="padding: 0 8px; margin: 0 4px; "> <span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">What happens when the XML doesn't embed meridian flip information? We retain the previous state?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D18235#inline-100664">View Inline</a><span style="color: #4b4d51; font-weight: bold;">capture.cpp:4597</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">reply</span><span class="p">.</span><span class="n">error</span><span class="p">().</span><span class="n">type</span><span class="p">()</span> <span style="color: #aa2211">!=</span> <span class="n">QDBusError</span><span style="color: #aa2211">::</span><span class="n">NoError</span><span class="p">)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">qCCritical</span><span class="p">(</span><span class="n">KSTARS_EKOS_CAPTURE</span><span class="p">)</span> <span style="color: #aa2211"><<</span> <span class="n">QString</span><span class="p">(</span><span style="color: #766510">"Warning: setting meridian flip request received DBUS error: %1"</span><span class="p">).</span><span class="n">arg</span><span class="p">(</span><span class="n">QDBusError</span><span style="color: #aa2211">::</span><span class="n">errorString</span><span class="p">(</span><span class="n">reply</span><span class="p">.</span><span class="n">error</span><span class="p">().</span><span class="n">type</span><span class="p">()));</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">We'll perhaps need a UI feedback here. No issues.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D18235#inline-100666">View Inline</a><span style="color: #4b4d51; font-weight: bold;">mount.cpp:46</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; "><span style="color: #304a96">#define ABORT_DISPATCH_LIMIT 3</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">#define MF_DELAY 12000</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Rationale?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D18235#inline-100669">View Inline</a><span style="color: #4b4d51; font-weight: bold;">mount.cpp:502</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">setInitialHA</span><span class="p">((</span><span class="n">sgn</span> <span style="color: #aa2211">==</span> <span style="color: #766510">'-'</span> <span style="color: #aa2211">?</span> <span style="color: #aa2211">-</span><span style="color: #601200">1</span><span style="color: #aa2211">:</span> <span style="color: #601200">1</span><span class="p">)</span> <span style="color: #aa2211">*</span> <span class="n">ha</span><span class="p">.</span><span class="n">Hours</span><span class="p">());</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">delete</span> <span class="n">currentTargetPosition</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">currentTargetPosition</span> <span style="color: #aa2211">=</span> <span style="color: #aa4000">new</span> <span class="n">SkyPoint</span><span class="p">(</span><span class="n">telescopeCoord</span><span class="p">.</span><span class="n">ra</span><span class="p">(),</span> <span class="n">telescopeCoord</span><span class="p">.</span><span class="n">dec</span><span class="p">());</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Small interval where currentTargetPosition is undefined.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D18235#inline-100667">View Inline</a><span style="color: #4b4d51; font-weight: bold;">mount.cpp:886</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">initialHA</span><span class="p">()</span> <span style="color: #aa2211">>=</span> <span style="color: #601200">0</span> <span style="color: #aa2211">||</span> <span class="n">meridianFlipCheckBox</span><span style="color: #aa2211">-></span><span class="n">isChecked</span><span class="p">()</span> <span style="color: #aa2211">==</span> <span style="color: #304a96">false</span><span class="p">)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">There might be something here. If the mount parking position is misplaced, it may happen that initialHA is positive in kstars, but that the mount is still on the wrong side. Do we avoid flipping still? Arguable.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D18235#inline-100668">View Inline</a><span style="color: #4b4d51; font-weight: bold;">mount.cpp:903</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">setMeridianFlipStatus</span><span class="p">(</span><span class="n">MF_PLANNED</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="p">}</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Even if the flip checkbox is unset?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D18235#inline-100670">View Inline</a><span style="color: #4b4d51; font-weight: bold;">mount.cpp:945</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">setMeridianFlipStatus</span><span class="p">(</span><span class="n">MF_RUNNING</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">slew</span><span class="p">(</span><span class="n">currentTargetPosition</span><span style="color: #aa2211">-></span><span class="n">ra</span><span class="p">().</span><span class="n">Hours</span><span class="p">(),</span> <span class="n">currentTargetPosition</span><span style="color: #aa2211">-></span><span class="n">dec</span><span class="p">().</span><span class="n">Degrees</span><span class="p">());</span>
</div><div style="padding: 0 8px; margin: 0 4px; "> <span style="color: #aa4000">return</span> <span style="color: #304a96">true</span><span class="p">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">We'd require error management here.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D18235#inline-100665">View Inline</a><span style="color: #4b4d51; font-weight: bold;">mount.h:59</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">typedef</span> <span style="color: #aa4000">enum</span> <span class="p">{</span> <span class="n">MF_IDLE</span><span class="p">,</span> <span class="n">MF_PLANNED</span><span class="p">,</span> <span class="n">MF_WAITING</span><span class="p">,</span> <span class="n">MF_ACCEPTED</span><span class="p">,</span> <span class="n">MF_RUNNING</span><span class="p">,</span> <span class="n">MF_COMPLETED</span><span class="p">,</span> <span class="n">MF_ERROR</span> <span class="p">}</span> <span class="n">MeridianFlipStatus</span><span class="p">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Aren't those name contexts clashing with Capture? It could be easy to mix them up.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R321 KStars</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D18235">https://phabricator.kde.org/D18235</a></div></div><br /><div><strong>To: </strong>wreissenberger, mutlaqja, TallFurryMan<br /><strong>Cc: </strong>kde-edu, narvaez, apol<br /></div>