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

[2.0.x] L6470 - add powerSTEP01 support (replaces PR #13053) #13498

Conversation

Bob-the-Kuhn
Copy link
Contributor

This PR replaces PR #13053. There are no functional differences between the code in the two PRs.

This PR uses the "helper" method of linking external SPI routines to the Arduino-L6470 library.

See PR #13053 for functionality information.

This PR requires a change to the Arduino L6470 library. Here are the new library files.
L6470 (helper).zip

@thinkyhead
Copy link
Member

I'll bring the library files over very soon, if you haven't already submitted a PR to my fork. I'm going to be on the road to MRRF so I may be slow to fully evaluate this, but I will be checking in periodically.

@Bob-the-Kuhn
Copy link
Contributor Author

Plans?

Would you like me to do something?

Bob-the-Kuhn and others added 21 commits May 17, 2019 23:35
Need to clear the powerup/reset error bits before can set slew rate on L6480/powerSTEP01
Change the way the array axis_mon is defined and how it's passed to the get_user_input function.
FULLY FUNCTIONAL using optional external SPI linked via  code within Marlin classes.

1) Went back to the handler/pointer method
2) removed helper class from L6470.h
3) renamed L6470_Marlin class to L64XX_MARLIN
4) removed all helper code and renamed anything that had "helper" in it.
Need to clear the powerup/reset error bits before can set slew rate on L6480/powerSTEP01
Change the way the array axis_mon is defined and how it's passed to the get_user_input function.
FULLY FUNCTIONAL using optional external SPI linked via  code within Marlin classes.

1) Went back to the handler/pointer method
2) removed helper class from L6470.h
3) renamed L6470_Marlin class to L64XX_MARLIN
4) removed all helper code and renamed anything that had "helper" in it.
Mostly fixing incomplete conversions and upper/lower case issues
Bob-the-Kuhn and others added 17 commits May 17, 2019 23:36
Need to clear the powerup/reset error bits before can set slew rate on L6480/powerSTEP01
Change the way the array axis_mon is defined and how it's passed to the get_user_input function.
FULLY FUNCTIONAL using optional external SPI linked via  code within Marlin classes.

1) Went back to the handler/pointer method
2) removed helper class from L6470.h
3) renamed L6470_Marlin class to L64XX_MARLIN
4) removed all helper code and renamed anything that had "helper" in it.
Mostly fixing incomplete conversions and upper/lower case issues
Configuration.h & Configuration_adv.h
  1. Remove test settings
  2. Merge in L6480 & powerSTEP01 items

M122 - remove unused line

u8g_com_HAL_DUE_st7920_sw_spi.cpp
pins_RAMPS_FD_V2.h
  used latest version from Bugfix

stepper_indirection.h - manually rebase

L6470_Marlin.cpp - use version from PR
@Bob-the-Kuhn
Copy link
Contributor Author

This PR is now up to date with the current 2.0.x.

It has been successfully tested.

@Bob-the-Kuhn
Copy link
Contributor Author

If wanted, I can squash all the commits down to one. There's so many false starts and rebases that I doubt the it's useful to keep access to the individual commits.

@Bob-the-Kuhn
Copy link
Contributor Author

This has been hanging around for 3 months. Time to kill it.

@thinkyhead
Copy link
Member

thinkyhead commented Jun 14, 2019

Old PRs don't need to be killed. I can keep them up to date as I get around to them. I have been trying to clear PRs but there are a few developers who feel the need to submit 2-3 non-essential UI enhancements a week, and this is killing our productivity in supporting hardware. I very much want to get this L6470 stuff merged whether it is "perfect" or not.

@Bob-the-Kuhn
Copy link
Contributor Author

I don't see a reason to invest any more work into these products. The TMC5160 is a superior solution in every way with the possible exception of the powerSTEP01 when applied to motors larger than NEMA 23.

Along that same line I'm wondering if we should just drop the support for this product line completely. I'm only aware of two people that tried to use these drivers and I was able to pursuade one of them to go with TMC drivers.

If you see value in merging this PR then I'm willing to give it one more shot.

I'm going to be travelling for the next 3 weeks so it'll probably be a month before I have time to pull the hardware out of storage and start the rebase/test process.

@thinkyhead
Copy link
Member

thinkyhead commented Jun 21, 2019

I agree it's not a vital feature, and we can always revisit it later on. Happy travels! We'll just keep sweating it out here in Austin.

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

Successfully merging this pull request may close these issues.

2 participants