indilib / indi-3rdparty

INDI 3rd Party drivers repository
https://www.indilib.org/devices.html
GNU Lesser General Public License v2.1
124 stars 208 forks source link

Adding Pololu Tic Focuser into INDI drivers #883

Open bzizou opened 8 months ago

bzizou commented 8 months ago

Integration of the driver for the DIY Pololu Tic Focuser from @sebo-b https://github.com/sebo-b/TicFocuser-ng

I tested the full kstars->indi->ticfocuser chain with NiXOS and this branch: https://github.com/bzizou/nixpkgs/tree/ticfocuser

As I only tested the "pololu-tic" connection, this driver only builts when the pololu-tic library is found on the system (https://github.com/pololu/pololu-tic-software) which is the case of the master branch of nixpkgs with the dependency "pololu-tic" (as seen here https://github.com/bzizou/nixpkgs/blob/ticfocuser/pkgs/development/libraries/science/astronomy/indilib/indi-3rdparty.nix) Once the pololu-tic focuser merged into Indi, I'll do a PR to NiXOS and this system will then be ready to use it with no effort.

I really hope that this great project could be integrated into Indi, as I know that I'm not the only one using this DIY focuser that is very easy to build.

sebo-b commented 8 months ago

LibUSB should work as well as Pololu Tic library - it is based on Pololu Arduino Library. In that case there is no dependency on Pololu Tic Library.

bzizou commented 7 months ago

LibUSB should work as well as Pololu Tic library - it is based on Pololu Arduino Library. In that case there is no dependency on Pololu Tic Library.

OK, have to test this. Then, what do you suggest?

By the way @sebo-b , many thanks for that great work! I'm using Tic-Focuser-Ng since a year and it was very easy to build and cheap!

knro commented 7 months ago

I concur. Having this based on libusb would simplify dependencies greatly.

bzizou commented 7 months ago

So, just checked the build without pololu tic library and it's ok. Then I just forced-pushed a version that does not enforce the use of this library.

knro commented 7 months ago

@bzizou Please check the build errors above

bzizou commented 7 months ago

@bzizou Please check the build errors above

Yes, I saw that. Warnings about deprecations turned into errors. That should be easy to fix.

bzizou commented 7 months ago

Actually, I'm not a CPP expert. Can someone help me to fix this:

'void INDI::DefaultDevice::defineText(ITextVectorProperty*)' is deprecated: Use defineProperty(INDI::Property &)

from this code line:

       m_Device->defineText(&TicSerialNumberTP);
knro commented 7 months ago

You need to use new-style INDI property as I commented above. It's fairly easy to use.

knro commented 7 months ago

@bzizou Do you need any help? INDI 2.0.6 is set to release in a week. Hopefully we can include this driver by then.

bzizou commented 7 months ago

@bzizou Do you need any help? INDI 2.0.6 is set to release in a week. Hopefully we can include this driver by then.

To be honest, yes... The code has not been written by me and I'm not a C++ developer (I've been a little, far away in the past ;-) ), so I don't understand the other errors I have when I do the suggested changes into the header files. For example:

Here's my patch:

diff --git a/indi-ticfocuser/TicFocuser.h b/indi-ticfocuser/TicFocuser.h
index 860cd340..e1206a47 100644
--- a/indi-ticfocuser/TicFocuser.h
+++ b/indi-ticfocuser/TicFocuser.h
@@ -62,10 +62,10 @@ class TicFocuser : public INDI::Focuser
         FocusDirection lastFocusDir; //< used to identify direction reversal (for backlash)

         ISwitch FocusParkingModeS[2];
-        ISwitchVectorProperty FocusParkingModeSP;
+        INDI::PropertySwitch FocusParkingModeSP {1};

         ISwitch EnergizeFocuserS[2];
-        ISwitchVectorProperty EnergizeFocuserSP;
+        INDI::PropertySwitch EnergizeFocuserSP {1};

         enum InfoTab {
             VIN_VOLTAGE,
@@ -78,10 +78,10 @@ class TicFocuser : public INDI::Focuser
         };

         IText InfoS[InfoTabSize] = {};
-        ITextVectorProperty InfoSP;
+        INDI::PropertyText InfoSP {1};

         IText* InfoErrorS;
-        ITextVectorProperty InfoErrorSP;
+        INDI::PropertyText InfoErrorSP {1};
 };

 #endif // TICFOCUSER_H
diff --git a/indi-ticfocuser/connection/BluetoothConnection.h b/indi-ticfocuser/connection/BluetoothConnection.h
index 087b1402..33b30973 100644
--- a/indi-ticfocuser/connection/BluetoothConnection.h
+++ b/indi-ticfocuser/connection/BluetoothConnection.h
@@ -52,7 +52,7 @@ class BluetoothConnection:
         bool callHandshake();

         IText BtMacAddressT[1] {};
-        ITextVectorProperty BtMacAddressTP;
+        INDI::PropertyText BtMacAddressTP {1};

         std::string requiredBtMacAddress;

diff --git a/indi-ticfocuser/connection/UsbConnectionBase.h b/indi-ticfocuser/connection/UsbConnectionBase.h
index 1137a043..b672d99d 100644
--- a/indi-ticfocuser/connection/UsbConnectionBase.h
+++ b/indi-ticfocuser/connection/UsbConnectionBase.h
@@ -47,7 +47,7 @@ class UsbConnectionBase:
     protected:

         IText TicSerialNumberT[1] {};
-        ITextVectorProperty TicSerialNumberTP;
+        INDI::PropertyText TicSerialNumberTP {1};

         std::string requiredSerialNumber;

And I get such errors:

/build/indi-3rdparty/indi-ticfocuser/TicFocuser.cpp: In member function 'virtual bool TicFocuser::initProperties()':
/build/indi-3rdparty/indi-ticfocuser/TicFocuser.cpp:112:25: error: 'INDI::PropertyView<T>* INDI::PropertyBasic<T>::operator&() [with T = _ISwitch]' is protected within this context
  112 |     IUFillSwitchVector(&FocusParkingModeSP,FocusParkingModeS,2,getDeviceName(),"FOCUS_PARK_MODE","Parking Mode",OPTIONS_TAB,IP_RW,ISR_1OFMANY,60,IPS_IDLE);
bzizou commented 7 months ago

Maybe @sebo-b , the original author, can help?

sebo-b commented 7 months ago

Hi - I will try to help, but I need to prepare the whole setup as I was not working on that code for a few years. Give me a week or so :)

bzizou commented 7 months ago

Hi - I will try to help, but I need to prepare the whole setup as I was not working on that code for a few years. Give me a week or so :)

Great! By "the whole setup", do you mean the hardware? As soon as the code compiles without errors, I Can do the functional tests on the whole setup if you want, just Ask 😉

sebo-b commented 7 months ago

By the whole setup, I meant the build environment. I was hoping that you can test it :)

sebo-b commented 7 months ago

@bzizou I have updated the project to avoid the use of the deprecated functions. Please compile and test it in the setup. It will be best if you compile both Pololu and LibUSB interfaces and test both. (the new code is in my original repo).

@knro I started moving the old property interfaces to the new ones, actually, I have almost finished it, but then I realized that it doesn't make too much sense in the current state of migration. Most of the drivers are still not updated, Connection::Interface and INDI::Focuser (base classes I use) are not migrated, so the old API will be mixed with the new one in multiple places. I will migrate when it becomes required as I assume it is not required now.

knro commented 7 months ago

It is required now for all new drivers. It will take a while to migrate all the other base classes and 3rd party drivers.

bzizou commented 7 months ago

@bzizou I have updated the project to avoid the use of the deprecated functions. Please compile and test it in the setup. It will be best if you compile both Pololu and LibUSB interfaces and test both. (the new code is in my original repo).

OK, I'll test this asap (I'm currently under heavy load at work, and I hope that I'll have some time during the evenings this week)

sebo-b commented 7 months ago

@bzizou thx

@knro I see, so I will finish the refactoring to the new APIs. As I'm not sure if I will be able to fully test it, would you be able to review the changes?

knro commented 7 months ago

Yes sure.

knro commented 2 months ago

@sebo-b Can you please rebase?