Proposal to remove DEC PPEC training from EQMod driver

[xthestreams]

Describe the bug
From what I can see, none of the SkyWatcher mounts support PPEC on the DEC axis, however the driver panel provides affordances (buttons) for DEC PEC training and playback, which is misleading and incorrect.

Unless there is a good reason to keep them for a specific mount (in which case logic can be built for this specific variant), then I think it's best to remove it to reduce confusion.

To Reproduce
The buttons all work, but it's unclear what they actually do, given there is no actual PPEC to enable on DEC

Expected behavior
The buttons should return an error on mounts that don't support DEC PPEC (which from what I can see are all SkyWatcher mounts)

Screenshots
attached

Desktop (please complete the following information):

• OS: Stellarmate
• Version most recent

Log Files
Make sure to enable logging and include log files https://indilib.org/support/logs-submission.html

[knro]

@geehalel Any thoughts?

[geehalel]

Actually the PPEC feature is determined by polling the motor controller itself. Technically both axis have PPEC as the motor firmware is the same for RA and DEC axis. Being a driver I leave DEC PPEC only for completeness (it is there, why should I remove it? Ok, users are confused, I also asked myself the question).
On the other hand the control panel is generic and displays every INDI properties (which it should do). The problem is that it is used as a control panel for every kind of devices. Hence the UI is actually reported in the driver code (a bad idea IMHO).
We may comment the code to not define DEC PPEC if users are so much confused. If someday someone has a use case (Alt-Az tracking ? no idea) it would be easy to restore.

[xthestreams]

Thanks geehalel, and thanks for an amazing driver - I was using the mount model features yesterday and they're so good!

Given that I seem to be the only person who reported the PEC driver crash (without training) wtih the EQ8-R, maybe I am just one of the few people who uses PPEC, so obviously I am crazy! My perspective is that, unless the vendor supports it (ie: it's documented in their manual), then it's probably better to hide it or move it to an Advanced or Experimental features tab where it can be documented and bugs can be reported and managed effectively.

Conversely, perhaps as the Autohome function IS a supported part of the product it might not need all the warnings when activated.

Just thinking out loud and frankly trying to stop the people new to the hobby (like me) tripping over things that cause problems/confusion.

[knro]

So just added warning whenever it is activated?

[geehalel]

Concerning the crash (actually an unhandled error) when PPEC is engaged without training, I believe that there is now a way to test the training status in the features exposed by the motor firmware. But I don't know if this is present in every firmwares/models.
I'm not sure anything is "supported" in the indi-eqmod driver (product!), I added autohome blindly using an algorithm given by someone from ascom eqmod and tested it on a breadboard. Never seen autohoming in real. I am as new as you are here.
You may comment DEC PPEC or suppress PPEC completely, it is really an UI concern in my mind.

[knro]

The autohome functionality works flawlessly with my EQ8 and I've tried that many times. Never used PPEC so can't comment on that.

[xthestreams]

On the topic of whether to keep the DEC PPEC training or not, I personally think it's tricky. The mount can detect "home" in both axis so that would make sense, assuming it actually works (I am still having a random big where the mount slews off into crazy land when PPEC training completes - the Green Swamp Server people have also seen it - so it seems to be a SkyWatcher firmware bug - I've reported it but they follow up after their first reply).
It also assumes that there isn't so much backlash in DEC that it is actually useful, which I doubt, so perhaps just disables it?
As for homing - the code works great as does Jassem's new code, I would remove the warning there and make it a safe feature.
But I am also not a developer/coder! :-)

[k-ross]

I don't know how the EQ8-R or EQ8-Rh mounts work, I only know the EQ6-R Pro (that's what I have). Any commands to the motor controller regarding PEC ignore the axis parameter specified. So when the driver queries each axis does it have PEC, it says yes to both, because it ignores the axis parameter. Okay no big deal.

The problem comes in during autoguiding. When autoguiding, the driver turns off PEC, issues the guiding pulse, waits for it to complete, then turns PEC back on. You can probably see the problem. If we issue a guide pulse to Dec, it turns off PEC (for RA axis) for no good reason while guiding the Dec axis.

[k-ross]

Oh I just remembered one more problem. If the control panel shows RA and Dec PEC are active, and I say to myself "Hey, Dec PEC shouldn't be on" and I click the button to turn off Dec, the driver will issue a command to turn off PEC for the Dec axis. But the firmware ignores axis parameter, because there's only one PEC, so I think I've only turned off PEC for the Dec axis, but I've turned it off for RA axis, too. But the control panel will still show RA PEC is active, even though it isn't. This is a clear bug.

[knro]

@k-ross is there perhaps a simple fix to this?

[k-ross]

@knro I can take a look at cleaning that up.

[k-ross]

Okay I removed all references to PPEC on the Declination axis, since it doesn't make sense. Also, renamed all occurrences of "RA PPEC" to just "PPEC", since that's what most people call it. It compiles, and appears to work, but I can't actually test it under the stars for a few days, because of nothing but rain in the forecast.

If anyone cares to try it out before I create a PR, you can go to my fork, and checkout the "no_dec_ppec" branch.

Otherwise, once the clouds clear, and if it tests out okay, I'll create a PR.

[k-ross]

Finally a night where I can see the stars. The changes seem to check out, so a PR (#479) has been created.

[knro]

Great, will close this issue, please reopen if another creeps up.