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

Using map of minimum speed in ramp increments' computations. #333

Open
wants to merge 2 commits into
base: BartsFixedIntervalRamp
Choose a base branch
from

Conversation

TheAntTeam
Copy link

Hi!
Doing a bit of tests changing the configurations, we realized that the ramp intervals did not take in account the minimum speed.
When 'speed_map: 0' is not 0, the ramp intervals were larger then what we expected, so less intervals were needed to do a ramp and less time passed respect to the spinup_ms/spindown_ms.

Moreover, we noticed two additional things:

  1. when the 'speed_map: 0' is not 0, during the initial connection the spin ramps up from zero to the minimum speed value
  2. we tried to change "spinup_ms" and "spindown_ms" by terminal, and the value changed but were not really applied until when we re-uploaded the config file.

@bdring
Copy link
Owner

bdring commented Mar 12, 2022

Does this change completely fix the problems for you?

Many config changes do not take effect immediately. You need to save them back to the localfs with $CD=<your_file> and then restart.

@TheAntTeam
Copy link
Author

Yes, it does fix it. We tested with the CNC and the spindle ramps up with the correct timing after the correction.

I didn't know about the re-saving of the file, my bad, and thanks for your answer.

About the initial ramp, that goes from 0 to the minimumSpeed, it doesn't affect us, but we don't know if it is correct and if it could have any side effect, that's why we pointed it out.

@bdring
Copy link
Owner

bdring commented Mar 12, 2022

I think some people will want ramping to the minimum speed.

Your explanation is confusing. This code uses fixed intervals and determines the increments based on the total time.

What exactly did you not like about the original code? Ignore the details and explain at the machine level. like...

It takes long than it should
It cause this problem...

@MitchBradley
Copy link
Collaborator

Presumably some people will want ramp to minimum and others will not.

We are designing something by guessing what people will want. We have one concrete use case for now - limiting overcurrent from too-abrupt speed changes. Design by speculation has never aged well for me.

@bdring
Copy link
Owner

bdring commented Mar 12, 2022

As I understand the problem. Ramping to min when that is not needed just adds a little time. Not ramping to min when it is needed causes over-current or other stresses.

The case where I used this before was for a cheap DC spindle. The spindle was basically worthless at anything less than full power, but we needed to ramp it anyway. Min was also max.

image

@TheAntTeam
Copy link
Author

Hi,
sorry for our poor explanation.
we are now using this configuration:

PWM:
pwm_hz: 50
output_pin: gpio.32
direction_pin: NO_PIN
disable_with_s0: false
s0_with_disable: true
spinup_ms: 5000
spindown_ms: 5000
tool_num: 0
speed_map: 0=5.300% 1000=9.300%
use_ramp: true

in which speed_map start from 5.3% (that equal to 0 rpm), this start duty is required to correctly initialize the ESC.

At the connection with the serial port, the PWM ramps up from 0 to 5.3%, this takes time in which the machine doesn't respond. We don't know if this could be an issue, but, in our case, this initial ramping isn't necessary because the real spindle speed stay at 0. With the 615a92d commit we try to solve this issue.

@bdring
Copy link
Owner

bdring commented Mar 14, 2022

This appears to be a BESC speed controller. Why are you not using the BESC spindle class?

I can test with that.

It is possible we could change from Boolean use_ramp an Enumeration ramp of (Off, PWM, SpeedMap)
Those were just a SWAG at the Enum item names.

@MitchBradley
Copy link
Collaborator

The initial ramp is likely an artifact of the initial value of _current_duty , which is probably 0. I suspect that the best way to fix it is to do something in init() to force the duty cycle and _current_duty to _speeds[0].offset . That way ramp_speed will never encounter a duty cycle outside the speed map range. That fix would be appropriate for all spindle types.

@bdring
Copy link
Owner

bdring commented Mar 14, 2022

I'll test that against all the speed_map examples we have on the wiki.

I really think this example should have been done using the BESC. If there are special needs for that, they can be fixed in that init(). I was going to work on derived spindles after the base PWM one was working.

@MitchBradley
Copy link
Collaborator

I'll save you some time - the only one with a nonzero @0 entry is https://github.com/bdring/FluidNC/wiki/FluidNC-Spindle-Speed-Mapping#nonzero-spindle-at-s0

@MitchBradley
Copy link
Collaborator

Okay, so this is probably going to blow your mind but I was thinking the other day about how to do it more generally, by adding optional times to the speed map, e.g. 0=0%u200 0=20%u500d100 1000=50%u1000d2000 5000=100%u2000d3000 .

That means that enabling takes 200 ms, going to the initial 20% speed takes an additional (500 - 200)=300 ms, the 20%-50% segment takes (1000-500)=500ms, etc. Spin down uses the d values. If u and d are not supplied anywhere, use 0 at the beginning and spinup/down at the end.

@TheAntTeam
Copy link
Author

This appears to be a BESC speed controller. Why are you not using the BESC spindle class?

I can test with that.

It is possible we could change from Boolean use_ramp an Enumeration ramp of (Off, PWM, SpeedMap) Those were just a SWAG at the Enum item names.

Hi Bart.
We have seen that in the BESC the frequency is fixed to 50 Hz, and since we test different ESCs and we had issues with the BLHELI since we needed more than 1 kHz, we used the more generic class that is also more configurable from what we saw.
Of course, if it is easier to test with the BESC class it's good for us..

@bdring
Copy link
Owner

bdring commented Mar 20, 2022

It would have been very helpful from the beginning if we knew your problems were related to the nature of BESC spindles.

I would suggest that the BESC spindle be changed to allow the frequency you need. Do you have a reference link to the frequency required for BLHELI

@TheAntTeam
Copy link
Author

We used different ESCs, some with 50 Hz, some like with higher frequency like the BLHeli with 1 kHz or 2 kHz.

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.

3 participants