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

Spindle refactoring, issue with original Huanyang logic #18

Open
dresco opened this issue Mar 19, 2023 · 10 comments
Open

Spindle refactoring, issue with original Huanyang logic #18

dresco opened this issue Mar 19, 2023 · 10 comments

Comments

@dresco
Copy link
Contributor

dresco commented Mar 19, 2023

Looks like the refactoring has impacted speed control for original Huanyang VFDs..

Here it's setting spindle_hal->rpm_max to the configured RPM @ 50Hz, which is then enforced in spindle_set_rpm()

case VFD_GetMaxRPM50:
if(spindle_hal) {
spindle_hal->cap.rpm_range_locked = On;
spindle_hal->rpm_max = rpm_max50 = (float)((msg->adu[4] << 8) | msg->adu[5]);
}
break;

@terjeio
Copy link
Contributor

terjeio commented Mar 19, 2023

Should it be limited to the min value of rpm_max50 and the $30 setting for max RPM?

@dresco
Copy link
Contributor Author

dresco commented Mar 19, 2023

No, it's a bit odd the way it does it..

These are typical settings for a 24k Chinese CNC spindle;

PD005 = 400    max freq (hz)
PD011 = 120    min freq (hz)
PD144 = 3000   spindle speed at 50hz

As I understand it, this VFD uses Hz instead of RPM when requesting a speed, and the rpm_max50 (PD144) value is just used to calculate the required frequency for a requested RPM..

Fwiw, it still seems to work fine (min/max capped as expected) if I revert to this. I haven't checked whether it's the VFD doing the limiting, or the $30/$31 settings - I could check the modbus messages to be sure.

            case VFD_GetMaxRPM50:
                rpm_max50 = (msg->adu[4] << 8) | msg->adu[5];
                break;

@terjeio
Copy link
Contributor

terjeio commented Mar 19, 2023

spindle_hal->cap.rpm_range_locked = On; should override the $30 and $31 settings for the VFD - I'll recheck this (with my simulator).

@dresco
Copy link
Contributor Author

dresco commented Mar 20, 2023

Did a bit more checking here..

With the older code (just setting rpm_max50);

  • grblHAL bounds the values sent to the VFD within $30/$31 limits
  • If the VFD receives a request under it's lower bound (PD011) then it uses the PD011 value
  • If the VFD receives a request above it's upper bound (PD005) then it does not change speed at all

Can I ask what the use case for the new code is, vs just setting $30/$31? Am guessing multi spindle support, where you may have different values at run time?

I think if you wanted to get the actual allowable RPM from the v1 Huanyang, then would need to query PD005, PD011, and PD144 during init. Having all three, could then calculate the configured min/max RPM ..

@terjeio
Copy link
Contributor

terjeio commented Mar 20, 2023

If the VFD receives a request above it's upper bound (PD005) then it does not change speed at all

This is handled in the VFD - by ignoring the speed command. grblHAL (and Grbl) clips the PWM spindle speed at max allowed speed and IMO VFDs should do the same to be consistent.

Can I ask what the use case for the new code is, vs just setting $30/$31?

$30/$31 is for the PWM spindle, and I see no reason for using the settings when the limits can be read from the VFD. And especially so in a multi spindle setup that includes a PWM spindle. And if both $30/$31 and the VFD settings are used for limits they have to be kept in sync to be meaningful?

I think if you wanted to get the actual allowable RPM from the v1 Huanyang, then would need to query PD005, PD011, and PD144 during init. Having all three, could then calculate the configured min/max RPM ..

Agree. The PD144 value read is not the max RPM but rather a scaling factor? From the manuals I have:
"This is set according to the actual revolution of the motor. The displayed value is the same as this
set value. It can be used as a monitoring parameter, which is convenient to the user. This set value
corresponds to the revolution at 50Hz."
And the default value is 1440 as stated in the manual, not 3000?

@dresco
Copy link
Contributor Author

dresco commented Mar 20, 2023

If the VFD receives a request above it's upper bound (PD005) then it does not change speed at all

This is handled in the VFD - by ignoring the speed command. grblHAL (and Grbl) clips the PWM spindle speed at max allowed speed and IMO VFDs should do the same to be consistent.

Agreed, I think intuitively I was just expecting the VFD to change speed, but to clip it at the PD005 value - rather than do nothing..

I think if you wanted to get the actual allowable RPM from the v1 Huanyang, then would need to query PD005, PD011, and PD144 during init. Having all three, could then calculate the configured min/max RPM ..

Agree. The PD144 value read is not the max RPM but rather a scaling factor? From the manuals I have: "This is set according to the actual revolution of the motor. The displayed value is the same as this set value. It can be used as a monitoring parameter, which is convenient to the user. This set value corresponds to the revolution at 50Hz." And the default value is 1440 as stated in the manual, not 3000?

Yes, it's supposed to be the physical motor speed (dependent on # poles etc) for a 50hz input. For the typical Chinese spindles, this is 3000rpm, with a safe rpm range of 7,200 min to 24,000 max.

The Huanyang v1 code calculates the frequency to send like this lroundf(rpm * 5000.0f / (float)rpm_max50), noting the value expected by the VFD is 10x the actual Hz.

edit: oops - 100x the actual Hz, two extra zeros..

@terjeio
Copy link
Contributor

terjeio commented Mar 20, 2023

Something like this then?
huanyang.zip

@dresco
Copy link
Contributor Author

dresco commented Mar 20, 2023

Something like this then? huanyang.zip

Cool I'll give that a test, thanks!

Was also just wondering whether it's worth renaming VFD_GetMaxRPM50 for clarity? Perhaps just VFD_GetRPM50?

@dresco
Copy link
Contributor Author

dresco commented Mar 20, 2023

That seems to work well for me.

Can confirm it picks up the correct RPM limits from the VFD, and falls back to the $30/$31 values if spindle is offline during init.

Noticed a small typo on lines 87 & 91, the values are right but the comments are the wrong way round..

@terjeio
Copy link
Contributor

terjeio commented Mar 20, 2023

Thanks, I'll commit the changes soon. I have fixed the typos already, and renamed the enum and variable as well.

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

2 participants