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

Fysetc pins updates for RAMPS and SKR 1.3 #13963

Merged

Conversation

Bob-the-Kuhn
Copy link
Contributor

More Fysetc Minipanel pins updates as reported on PRs #13871 and #13956.

@thinkyhead
Copy link
Member

At some point you will want to get a fresh bugfix-2.0.x to use as a starting-point for new changes. This will make sure your working copy and your fork are both pointing to the correct HEAD commit:

git branch -D bugfix-2.0.x
git fetch upstream
git checkout upstream/bugfix-2.0.x -b bugfix-2.0.x
git push -f --set-upstream origin bugfix-2.0.x

Then when you use bugfix-2.0.x to start a new branch it is guaranteed to be up to date. You should repeat this same procedure on a daily basis to keep bugfix-2.0.x always in sync with upstream:

git fetch upstream
git checkout bugfix-2.0.x
git reset --hard upstream/bugfix-2.0.x
git push -f origin

If you utilize the Git Helper scripts included with Marlin (buildroot/share/git) then all these details are handled automatically. For example they always do git fetch upstream to make sure the latest code is being referenced. And, scripts like mfnew always refer to upstream/bugfix-2.0.x directly, since the local working copy or origin branch might be out of date. I did a live stream yesterday to demonstrate these scripts and a bit of PR management.

@thinkyhead
Copy link
Member

Commit messages like "pins updates" are unacceptable. Please make an effort to provide meaningful commit messages following the written guidelines at https://github.com/MarlinFirmware/Marlin/blob/1.1.x/.github/contributing.md#git-commit-messages.

@thinkyhead thinkyhead changed the title [2.0.x]More Fysetc Minipanel pins updates Fysetc pins updates for RAMPS and SKR 1.3 May 10, 2019
@Bob-the-Kuhn
Copy link
Contributor Author

Bob-the-Kuhn commented May 10, 2019

Yes, I've gotten lazy. Sorry about that.

Thanks for the pointers. I've copied them into my crib sheet.

What do you want to do about the L6470 PR #13498? Last night I grabbed a copy from the PR, rebased it and spent a few hours trying to fix the resulting compile errors.

I have mixed feelings about killing or completing that PR. The drivers provided few improvements in my system over what I was getting with A4988s so I moved on to the TMC 5160. On the other hand there is one (expensive) controller out there that uses the L6470.

@thinkyhead thinkyhead merged commit 75eca5c into MarlinFirmware:bugfix-2.0.x May 11, 2019
@thinkyhead
Copy link
Member

What do you want to do about the L6470 PR #13498? Last night I grabbed a copy from the PR, rebased it and spent a few hours trying to fix the resulting compile errors.

I hope we can get the L6470 code sorted out and making sense. I've recently been looking at some alternate design patterns that may be better for code that's intended to be used like a library in the Arduino context (i.e., not linking a binary but including source). We face some tricky challenges when it comes to external libraries:

  • Providing such things as an alternative SPI bus, Serial Port, or Interrupt Vector
  • Providing callbacks that are integrated well with the library class, or at least well-specified.

The ExtUI approach uses a namespace and defines function interfaces that need to be implemented. The L6470 library used to take a similar approach, except using a global function with no namespace. In making L6470 into a more strict 'singleton' class, I wanted to find some way where we could leave the required methods undefined, and there would be some sensible default behavior. But as we saw, there are challenges to almost any approach.

So I've been looking at the non-virtual interface pattern, where a public interface is used to conceal a private interface, and the sub-classes are supposed to implement the private interface. In this way all code is routed through the root class's public interface.

And I'm trying to think of ways that C++ templates can help.

Some combination of approaches will probably help us to corral these stepper libraries. The trickiest part is just —as mentioned— getting them to hook into whatever SPI or serial bus we have available, and making the interfaces standard in some way. The approach that TMCStepper takes is probably the cleanest, but there's room for improvement with that library too.

ozgunawesome pushed a commit to ozgunawesome/Marlin that referenced this pull request May 29, 2019
@Bob-the-Kuhn Bob-the-Kuhn deleted the more-Fysetc-pins-updates branch January 15, 2020 09:10
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.

2 participants