Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect channel addressing for Thorlabs Piezo Inertia Actuator #372

Open
GarbatyGrabarz opened this issue Sep 30, 2022 · 11 comments
Open

Comments

@GarbatyGrabarz
Copy link

GarbatyGrabarz commented Sep 30, 2022

The issue applies to sub-messages where the channel ID is passed as part of data packet. In those cases the channels should be addressed as 1, 2, 4, and 8 rather than 1, 2, 3, 4. The only exception to this is enabled_single method which uses addresses 1-4 (plus 0, 5, and 6 for special cases). Affected methods: drive_op_parameters, jog_parameters, move_abs

I have tested this only for APTPiezoInertiaActuator class using KIM101 so I am not certain if this is not more general issue.

[EDIT] I have realized that Thorlabs documentation for move_jog suggest it should function with addresses 1-4 but in practice they also need to be addressed as 1, 2, 4, 8

@trappitsch
Copy link
Contributor

Interesting, I see this in the docs now too. The KIM101 support was included in #242. I remember that I had a KIM101 plus one mirror mount, but am not sure anymore if I ran it under enabled_single or not (probably I did when I tested the different channels with the hardware).

Thorlabs equipment, especially these cubes, are quite painful since there's tons of errors in the docs, so I'm not at all surprised that the move_jog is using the wrong channel addresses as you pointed out.

I currently only have a KDC101 handy with a rotation stage. I can have a look at a fix, or do you have a fix ready to go @GarbatyGrabarz? I'm a bit strapped on time at the moment, but might be able to check this out over the weekend... If that would be of interest, it would be great @GarbatyGrabarz if you could test any fix in enabled_single and enabled_multi mode. I can also test any fix with the KDC101, however, it only has one channel and channel 1 will stay channel 1 :)

Great to see somebody else using the APT Motor controllers with ik, I'm always interested in making them better because I have some laser automation down the line I'd like to implement. The more users, the merrier!

@GarbatyGrabarz
Copy link
Author

I did a quick fix by replacing self._idx_chan with 2**(self._idx_chan-1) in affected areas:

  • drive_op_parameters - data in a packet, line 495
  • jog_parameters - data in a packet, line 719
  • move_abs - data in a packet, line 807
  • position_count (forgot to mention this one) - data in a packet, line 775
  • move_jog - param1 , line 846

enabled_single and enabled_multi works without problem for now :)

However, my knowledge (or more importantly: my confidence) of programing is too low to try an actual commit (especially with strict requirements for this repo)

@trappitsch
Copy link
Contributor

No worries, I'm glad you have a quick fix that works. I can prepare a fix and then hopefully you could give me a hand with testing it with hardware. Would be great to have this all working in ik.

Thanks for the pointers above on where you changed parameters such that they work! This is definitely helpful as a start.

@GarbatyGrabarz
Copy link
Author

When your fix will arrive I will gladly test it :)

@scasagrande
Copy link
Contributor

Thanks for the report @GarbatyGrabarz !

@trappitsch
Copy link
Contributor

@GarbatyGrabarz: I created a fix, could you try it out with the KIM101? If you just cloned the Github branch and installed ik from there editable, you could easily just checkout my BF branch. The PR is here #373. Would be great to test with the hardware :)

The fix is actually very tiny, I'm simply setting self._idx_chan = 2**idx_chan on line 48 in thorlabsapt.py. The larger changes are the tests :)

@GarbatyGrabarz
Copy link
Author

I can do all the tests on Monday. However, what you've described will fix this issue but likely cause another one, a reverse one so to say. Except for the described above, in all other cases the channels should be addressed as 1, 2, 3, 4. For example enabling single channel uses channel addresses 1-4 (0 for disable, 5 for channels 1+2, and 6 for channels 3+4)

@trappitsch
Copy link
Contributor

Thanks for the input. I need to read up on this a bit more, but wouldn't your fix above then have the same problem?

@GarbatyGrabarz
Copy link
Author

That's where Thorlabs mess come to play. It seem that when you need to address the channel in the 6 byte header then you use 1-4, however when you need to address the channel in the data packet that follows the header you need to use 1, 2, 4, and 8. It's not a general rule described anywhere but if you browse through the sub-messages definitions for KIM101 they all have the addressing specifically described

@trappitsch
Copy link
Contributor

Need to do some more reading of the manual here definitely. I have a couple of very busy weeks ahead, so it will take a bit of time, but I won't forget about it. As long as your workaround is functional for now, I hope this is okay @GarbatyGrabarz

@GarbatyGrabarz
Copy link
Author

Sure, no worries. I am afraid there is no one neat solution. All the sub-messages are implemented on case-to-case basis, not as a method. Having individual sub-message ID and other stuff, what I propose may be the only way to go. The only thing worth checking is whether this sub-message mess happens for other devices.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants