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

Nanotec, Phytron and Leadshine motors review #184

Merged
merged 33 commits into from
Aug 25, 2023
Merged

Nanotec, Phytron and Leadshine motors review #184

merged 33 commits into from
Aug 25, 2023

Conversation

cdn-lnk
Copy link
Contributor

@cdn-lnk cdn-lnk commented Jul 28, 2023

Hi @anderssandstrom ,

This is my last week at ESS as a consultant.
I am sending the reviews and fixes I had locally before I return the computer.

They mostly focus on Nanotec, Phytron and Leadshine motors with EL7037, EL7047, EL7041 and EL7041-0052 modules (these are the ones I am using lately).

My recommendation is to go commit by commit instead of trying to review everything at once since there are around 150 files 😬 (I organized the commits by motor).

I didn't have time to implement voltage and current macros for all of them but I managed to import the EL7041 configs from EL7047 on all of them and I am using links when the config files would be identical.

I also implemented a voltage check script, but I am not calling it from any motor (yet).

Note: I changed the default voltage on some motors. I kept these in a separate commit in case you want to cherry pick them out. (The modules are limited to 48V, see Javier comment below).

Note2: The config for the Leadshine 57HS09 was quite off!!!

FYI: @mac-kan, @javicereijo, @lucianocguedes, @laispc


Useful links:

I could not find the Leadshine manual online, I am uploading it in this comment.
57hsxxd-hqm.pdf (thanks Laís).

@javicereijo
Copy link
Contributor

@cdn-lnk please check the config for the motors that work at more than 48 V, even if the motors accept more voltage the motor drives (at least the BEckhoff ones that we use in the accelerator) cannot provide more than 48 V.

Carlos Doro Neto added 29 commits July 28, 2023 17:24
Default voltage increased to 48V from 24V
It is just extra maintenance and github does a better job at keeping track of who changed what
Increases max current to 2.5A from 1.2A
Increases max current to 1.2A
Fixes resistence to 3.9 Ohms
…0FH1-0000.cmd and ecmcEP7047-1032-Motor-Owis-SM24x.cmd
Copy link

@mac-kan mac-kan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cdn-lnk
general/chkValidCurrentSetOrDie.cmd: Correct spelling - stanby->standby

I saw your commit about removing your name from the motor files since this is taken care of in git anyhow. The convention in this repo has been to have an author stated, will this change for all files or is there a reason for keeping it?

Since we are updating a lot of motor config scripts, please fix the spelling of Parmetrization->Parametrization

@cdn-lnk
Copy link
Contributor Author

cdn-lnk commented Aug 9, 2023

Hej @mac-kan,

general/chkValidCurrentSetOrDie.cmd: Correct spelling - stanby->standby

Fixed!

Since we are updating a lot of motor config scripts, please fix the spelling of Parmetrization->Parametrization

Also fixed! Thanks.

I saw your commit about removing your name from the motor files since this is taken care of in git anyhow. The convention in this repo has been to have an author stated, will this change for all files or is there a reason for keeping it?

I don't have a strong opinion on this.
I can re-add myself as author if needed/necessary/wanted.

@anderssandstrom anderssandstrom merged commit 14f9717 into paulscherrerinstitute:master Aug 25, 2023
2 checks passed
@anderssandstrom
Copy link
Contributor

Looks nice! Thanks

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

Successfully merging this pull request may close these issues.

4 participants