×

INDI Library v2.0.6 is Released (02 Feb 2024)

Bi-monthly release with minor bug fixes and improvements

Variable Reuse AbortSP

  • Posts: 161
  • Thank you received: 39
So as part of trying to implement INDI::FocuserInterface as part of OnStep (lx200 derivative) I've found I've got an issue

I should be able to have INDI::FocuserInterface also be imported to give the focuser interface, along with the INDI::Telescope (via lx200 generic)

Unfortunately, both use AbortSP. Which gives me the make output (1) below

I'm a rather rusty with C++, but this seems like an inheritance problem which is solved far more simply by changing the variable to FocusAbortSP, as trying to explicitly change them is a problem. Especially since FocuserInterface is supposed to be added to other devices.

I did a little checking and it's not limited to Telescopes, see (2) below. Looking at other drivers, there are some reuses, but they seem confined to derivatives of Telescope only, excluding the auxiliary/snapcap (3) (4)

So what I think would be best would be to rename the other couple in the base libraries to DomeAbortSP, FocuserAbortSP. The one I'm conflicted on, is the Telescope AbortSP as it's used in other drivers, but I do think it would be best to make sure there's not a conflict.

Does that make sense, or am I making something far harder than necessary? (I've done it in my own tree without seeming problems, other than getting the Focuser I want to work..) Thoughts?



(1) /home/james/git/indi/libindi/drivers/telescope/lx200_OnStep.cpp: In member function ‘virtual bool LX200_OnStep::Park()’:
/home/james/git/indi/libindi/drivers/telescope/lx200_OnStep.cpp:1015:20: error: reference to ‘AbortSP’ is ambiguous
IDSetSwitch(&AbortSP, "Abort slew failed.");
^~~~~~~
In file included from /home/james/git/indi/libindi/drivers/telescope/lx200_OnStep.h:31:0,
from /home/james/git/indi/libindi/drivers/telescope/lx200_OnStep.cpp:25:
/home/james/git/indi/libindi/libs/indibase/indifocuserinterface.h:180:27: note: candidates are: ISwitchVectorProperty INDI::FocuserInterface::AbortSP
ISwitchVectorProperty AbortSP;
^~~~~~~
In file included from /home/james/git/indi/libindi/drivers/telescope/lx200generic.h:24:0,
from /home/james/git/indi/libindi/drivers/telescope/lx200_OnStep.h:28,
from /home/james/git/indi/libindi/drivers/telescope/lx200_OnStep.cpp:25:
/home/james/git/indi/libindi/libs/indibase/inditelescope.h:617:27: note: ISwitchVectorProperty INDI::Telescope::AbortSP

(2)

indibase/indidome.cpp: IUFillSwitchVector(&AbortSP, AbortS, 1, getDeviceName(), "DOME_ABORT_MOTION", "Abort Motion", MAIN_CONTROL_TAB,
indibase/indidome.cpp: defineSwitch(&AbortSP);
indibase/indidome.cpp: deleteProperty(AbortSP.name);
indibase/indidome.cpp: if (!strcmp(name, AbortSP.name))
indibase/indidome.cpp: IUResetSwitch(&AbortSP);
indibase/indidome.cpp: AbortSP.s = IPS_OK;
indibase/indidome.cpp: AbortSP.s = IPS_ALERT;
indibase/indidome.cpp: IDSetSwitch(&AbortSP, nullptr);
indibase/indidome.cpp: return (AbortSP.s == IPS_OK);
indibase/indidome.h: ISwitchVectorProperty AbortSP;
indibase/indifocuser.cpp: AbortSP.s = IPS_OK;
indibase/indifocuser.cpp: AbortSP.s = IPS_ALERT;
indibase/indifocuser.cpp: IDSetSwitch(&AbortSP, nullptr);
indibase/indifocuserinterface.cpp: IUFillSwitchVector(&AbortSP, AbortS, 1, m_defaultDevice->getDeviceName(), "FOCUS_ABORT_MOTION", "Abort Motion", groupName, IP_RW,
indibase/indifocuserinterface.cpp: m_defaultDevice->defineSwitch(&AbortSP);
indibase/indifocuserinterface.cpp: m_defaultDevice->deleteProperty(AbortSP.name);
indibase/indifocuserinterface.cpp: IUResetSwitch(&AbortSP);
indibase/indifocuserinterface.cpp: AbortSP.s = IPS_OK;
indibase/indifocuserinterface.cpp: AbortSP.s = IPS_ALERT;
indibase/indifocuserinterface.cpp: IDSetSwitch(&AbortSP, nullptr);
indibase/indifocuserinterface.h: ISwitchVectorProperty AbortSP;
indibase/inditelescope.cpp: IUFillSwitchVector(&AbortSP, AbortS, 1, getDeviceName(), "TELESCOPE_ABORT_MOTION", "Abort Motion", MAIN_CONTROL_TAB,
indibase/inditelescope.cpp: defineSwitch(&AbortSP);
indibase/inditelescope.cpp: defineSwitch(&AbortSP);
indibase/inditelescope.cpp: deleteProperty(AbortSP.name);
indibase/inditelescope.cpp: if (!strcmp(name, AbortSP.name))
indibase/inditelescope.cpp: IUResetSwitch(&AbortSP);
indibase/inditelescope.cpp: AbortSP.s = IPS_OK;
indibase/inditelescope.cpp: AbortSP.s = IPS_ALERT;
indibase/inditelescope.cpp: IDSetSwitch(&AbortSP, nullptr);
indibase/inditelescope.cpp: ISNewSwitch(getDeviceName(), AbortSP.name, states, names, 1);
indibase/inditelescope.h: ISwitchVectorProperty AbortSP;

(3) auxiliary/snapcap.cpp: IUFillSwitchVector(&AbortSP, AbortS, 1, getDeviceName(), "Abort", "", MAIN_CONTROL_TAB, IP_RW, ISR_1OFMANY, 0,
auxiliary/snapcap.cpp: defineSwitch(&AbortSP);
auxiliary/snapcap.cpp: deleteProperty(AbortSP.name);
auxiliary/snapcap.cpp: if (strcmp(AbortSP.name, name) == 0)
auxiliary/snapcap.cpp: IUResetSwitch(&AbortSP);
auxiliary/snapcap.cpp: AbortSP.s = Abort();
auxiliary/snapcap.cpp: IDSetSwitch(&AbortSP, nullptr);
auxiliary/snapcap.cpp: IDSetSwitch(&AbortSP, nullptr);
auxiliary/snapcap.h: ISwitchVectorProperty AbortSP;
telescope/ioptronHC8406.cpp: AbortSP.s = IPS_ALERT;
telescope/ioptronHC8406.cpp: IDSetSwitch(&AbortSP, "Abort slew failed.");
telescope/ioptronHC8406.cpp: AbortSP.s = IPS_OK;
telescope/ioptronHC8406.cpp: IDSetSwitch(&AbortSP, "Slew aborted.");
telescope/lx200ap.cpp: AbortSP.s = IPS_ALERT;
telescope/lx200ap.cpp: IDSetSwitch(&AbortSP, "Abort slew failed.");
telescope/lx200ap.cpp: AbortSP.s = IPS_OK;
telescope/lx200ap.cpp: IDSetSwitch(&AbortSP, "Slew aborted.");
telescope/lx200ap_experimental.cpp: AbortSP.s = IPS_ALERT;
telescope/lx200ap_experimental.cpp: IDSetSwitch(&AbortSP, "Abort slew failed.");
telescope/lx200ap_experimental.cpp: AbortSP.s = IPS_OK;
telescope/lx200ap_experimental.cpp: IDSetSwitch(&AbortSP, "Slew aborted.");
telescope/lx200ap_gtocp2.cpp: AbortSP.s = IPS_ALERT;
telescope/lx200ap_gtocp2.cpp: IDSetSwitch(&AbortSP, "Abort slew failed.");
telescope/lx200ap_gtocp2.cpp: AbortSP.s = IPS_OK;
telescope/lx200ap_gtocp2.cpp: IDSetSwitch(&AbortSP, "Slew aborted.");
telescope/lx200basic.cpp: AbortSP.s = IPS_ALERT;
telescope/lx200basic.cpp: IDSetSwitch(&AbortSP, "Abort slew failed.");
telescope/lx200basic.cpp: AbortSP.s = IPS_OK;
telescope/lx200basic.cpp: IDSetSwitch(&AbortSP, "Slew aborted.");
telescope/lx200generic.cpp: AbortSP.s = IPS_ALERT;
telescope/lx200generic.cpp: IDSetSwitch(&AbortSP, "Abort slew failed.");
telescope/lx200generic.cpp: AbortSP.s = IPS_OK;
telescope/lx200generic.cpp: IDSetSwitch(&AbortSP, "Slew aborted.");
telescope/lx200generic.cpp: AbortSP.s = IPS_ALERT;
telescope/lx200generic.cpp: IDSetSwitch(&AbortSP, "Abort slew failed.");
telescope/lx200generic.cpp: AbortSP.s = IPS_OK;
telescope/lx200generic.cpp: IDSetSwitch(&AbortSP, "Slew aborted.");
telescope/lx200gotonova.cpp: AbortSP.s = IPS_ALERT;
telescope/lx200gotonova.cpp: IDSetSwitch(&AbortSP, "Abort slew failed.");
telescope/lx200gotonova.cpp: AbortSP.s = IPS_OK;
telescope/lx200gotonova.cpp: IDSetSwitch(&AbortSP, "Slew aborted.");
telescope/lx200_OnStep.cpp: INDI::Telescope::AbortSP.s = IPS_ALERT;
telescope/lx200_OnStep.cpp: IDSetSwitch(&AbortSP, "Abort slew failed.");
telescope/lx200_OnStep.cpp: INDI::Telescope::AbortSP.s = IPS_OK;
telescope/lx200_OnStep.cpp: IDSetSwitch(&INDI::Telescope::AbortSP, "Slew aborted.");
telescope/lx200pulsar2.cpp: AbortSP.s = IPS_ALERT;
telescope/lx200pulsar2.cpp: IDSetSwitch(&AbortSP, "Abort slew failed.");
telescope/lx200pulsar2.cpp: AbortSP.s = IPS_OK;
telescope/lx200pulsar2.cpp: IDSetSwitch(&AbortSP, "Slew aborted.");
telescope/lx200pulsar2.cpp: AbortSP.s = IPS_ALERT;
telescope/lx200pulsar2.cpp: IDSetSwitch(&AbortSP, "Abort slew failed.");
telescope/lx200pulsar2.cpp: AbortSP.s = IPS_OK;
telescope/lx200pulsar2.cpp: IDSetSwitch(&AbortSP, "Slew aborted.");
telescope/lx200zeq25.cpp: AbortSP.s = IPS_ALERT;
telescope/lx200zeq25.cpp: IDSetSwitch(&AbortSP, "Abort slew failed.");
telescope/lx200zeq25.cpp: AbortSP.s = IPS_OK;
telescope/lx200zeq25.cpp: IDSetSwitch(&AbortSP, "Slew aborted.");

(4)

indi-nexstarevo/nexstarevo.cpp: AbortSP.s = IPS_OK;
indi-nexstarevo/nexstarevo.cpp: IUResetSwitch(&AbortSP);
indi-nexstarevo/nexstarevo.cpp: IDSetSwitch(&AbortSP, NULL);
6 years 1 day ago #24538

Please Log in or Create an account to join the conversation.

  • Posts: 161
  • Thank you received: 39

Replied by james_lan on topic Variable Reuse AbortSP

Here is the branch/commit: github.com/james-lan/indi/tree/abortchange that does what I'm describing.
6 years 1 day ago #24542

Please Log in or Create an account to join the conversation.

Replied by Jasem Mutlaq on topic Variable Reuse AbortSP

Just use it explicitly like this:
FI::AbortSP.s = IPS_OK;
Telescope::AbortSP.s = IPS_OK;
6 years 1 day ago #24556

Please Log in or Create an account to join the conversation.

  • Posts: 161
  • Thank you received: 39

Replied by james_lan on topic Variable Reuse AbortSP

@knro

I forgot to mention that I tried that, but all of the uses of it as a pointer will give an error, which in every class is protected. (See below) It's called that way for (based on a quick scan of my list) every driver and library class that uses it. If it's unambiguous, and called from the child's reference, as opposed to a parent class, that's not a problem, but when you've got multiple inherited variables, and try to access that way, it will fail.

That's a lot of what made me think renaming would be far easier. Especially in a class (FocuserInterface) which should be usable in others, but without renaming or brushing up on esoteric C++ can't be used. (If fixing the issue for one, why not fix it to make sure it's not an issue for others as well? For Onstep on a Mega, using one of the free stepper drivers to control a dome interface doesn't seem too far fetched to me.) As I'm a bit rusty in C++, fighting this before giving up and saying rename would be better took a while, in both searching out solutions to try, and reading through C++ books. As far as I can tell, there isn't a good solution, other than renaming. (Additionally, If it saves someone else who is rusty in C++ hours when they could be doing something like working on making sure it works, rather that (re-)learning the annoyance that is C++ inheritance, that's a win.)


Here's the build output from trying your solution:
[ 49%] Building CXX object CMakeFiles/indi_lx200generic.dir/drivers/telescope/lx200_OnStep.cpp.o
/home/james/git/indi/libindi/drivers/telescope/lx200_OnStep.cpp: In member function ‘virtual bool LX200_OnStep::Park()’:
/home/james/git/indi/libindi/drivers/telescope/lx200_OnStep.cpp:1010:31: error: ‘ISwitchVectorProperty INDI::Telescope::AbortSP’ is protected within this context
IDSetSwitch(&Telescope::AbortSP, "Abort slew failed.");
^~~~~~~
In file included from /home/james/git/indi/libindi/drivers/telescope/lx200generic.h:24:0,
from /home/james/git/indi/libindi/drivers/telescope/lx200_OnStep.h:28,
from /home/james/git/indi/libindi/drivers/telescope/lx200_OnStep.cpp:25:
/home/james/git/indi/libindi/libs/indibase/inditelescope.h:617:27: note: declared protected here
ISwitchVectorProperty AbortSP;
^~~~~~~
/home/james/git/indi/libindi/drivers/telescope/lx200_OnStep.cpp:1010:60: error: cannot convert ‘ISwitchVectorProperty INDI::Telescope::* {aka _ISwitchVectorProperty INDI::Telescope::*}’ to ‘const ISwitchVectorProperty* {aka const _ISwitchVectorProperty*}’ for argument ‘1’ to ‘void IDSetSwitch(const ISwitchVectorProperty*, const char*, ...)’
IDSetSwitch(&Telescope::AbortSP, "Abort slew failed.");
^
/home/james/git/indi/libindi/drivers/telescope/lx200_OnStep.cpp:1015:30: error: ‘ISwitchVectorProperty INDI::Telescope::AbortSP’ is protected within this context
IDSetSwitch(&Telescope::AbortSP, "Slew aborted.");
^~~~~~~
In file included from /home/james/git/indi/libindi/drivers/telescope/lx200generic.h:24:0,
from /home/james/git/indi/libindi/drivers/telescope/lx200_OnStep.h:28,
from /home/james/git/indi/libindi/drivers/telescope/lx200_OnStep.cpp:25:
/home/james/git/indi/libindi/libs/indibase/inditelescope.h:617:27: note: declared protected here
ISwitchVectorProperty AbortSP;
^~~~~~~
/home/james/git/indi/libindi/drivers/telescope/lx200_OnStep.cpp:1015:54: error: cannot convert ‘ISwitchVectorProperty INDI::Telescope::* {aka _ISwitchVectorProperty INDI::Telescope::*}’ to ‘const ISwitchVectorProperty* {aka const _ISwitchVectorProperty*}’ for argument ‘1’ to ‘void IDSetSwitch(const ISwitchVectorProperty*, const char*, ...)’
IDSetSwitch(&Telescope::AbortSP, "Slew aborted.");
^
CMakeFiles/indi_lx200generic.dir/build.make:470: recipe for target 'CMakeFiles/indi_lx200generic.dir/drivers/telescope/lx200_OnStep.cpp.o' failed
make[2]: *** [CMakeFiles/indi_lx200generic.dir/drivers/telescope/lx200_OnStep.cpp.o] Error 1
CMakeFiles/Makefile2:1366: recipe for target 'CMakeFiles/indi_lx200generic.dir/all' failed
make[1]: *** [CMakeFiles/indi_lx200generic.dir/all] Error 2
Makefile:129: recipe for target 'all' failed
make: *** [all] Error 2
6 years 1 day ago #24571

Please Log in or Create an account to join the conversation.

Replied by Jasem Mutlaq on topic Variable Reuse AbortSP

How about this:
IDSetSwitch(&(Telescope::AbortSP), "Slew aborted.");

FYI, the names were different before. I actually do not like we do multiple inheritance all over the place now. We could employ composition instead of multiple inheritance, and perhaps use C++11 lambdas instead of the forwarding functions, but a change of this scale requires a re-write of all drivers. So for now, the current structure works OK for most cases. Hopefully one day it can be updated to a better and modern OOP design.
The following user(s) said Thank You: james_lan
Last edit: 6 years 19 hours ago by Jasem Mutlaq.
6 years 19 hours ago #24576

Please Log in or Create an account to join the conversation.

Time to create page: 0.560 seconds