Beckhoff-USA-Community / SPT-Libraries

MIT License
70 stars 16 forks source link

FB_BasicAxis: MoveVelocity #8

Closed stefanspiegel closed 1 year ago

stefanspiegel commented 1 year ago

Hi all,

once again thanks for your great work! Really appreciate it.

I think I found a small bug in the MoveVelocity-Command of the FB_BasicAxis:

See attached image. Checked at version 3.0.7.

SPT_FB_BasicAxis_MoveVelocity

Best regards from switzerland, Stefan

nshiggins commented 1 year ago

Thanks for the kind words and also for taking the time to dig in.

The way MoveVelocity() works was actually intentional. I tried to make the method arguments as few as possible for readability but mainly to be less annoying to use. So MoveAbsolute just takes Position and AbortPrevious, MoveVelocity takes Velocity and AbortPrevious, etc. In general I think that once you set some dynamics properties you should then be able to make a move using a single method call. Obviously not universal, but trying.

I chose to let the Velocity argument be negative because it felt less cumbersome than typing out the MC_Direction enum. The reason I set the _Velocity property backer was for the scenario where you might move at a velocity until some condition is met, and then call MoveModulo to stop at a known position--doing it this way lets this happen with no step in the velocity setpoint, and also without revisiting the Velocity property--2 moves, 2 lines of code (well sorta).

You do raise a good point and I do need to add some code to check for < 0 in the Velocity property setter. I'll make a work item for this and incorporate into the next release.

Not sure if you saw or not, but all of the SPT_NC... libraries have been rolled into a single SPT_MotionControl library with v3.1. Same code, just less repos to maintain.

stefanspiegel commented 1 year ago

Thanks for the hint with the updated library contents. We will update our references accordingly. For me a negative Velocity is fine and your approach is ok. Also saving the _Velocity for later commands is ok, as long as it is documented.

Greetings from switzerland, Stefan

nshiggins commented 1 year ago

Will get this added to the docs soon--we are currently porting them from the wiki to a different system that should be easier to navigate and read.