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

Refactor brushless support #86

Closed
wants to merge 40 commits into from
Closed

Conversation

jedahan
Copy link
Collaborator

@jedahan jedahan commented Sep 12, 2023

Note: This is a draft PR until I get back to my axidraw (~Sep 20)

This is based off the esbuild branch, and contains a mix of things.

Apologies for mixing a few different things, hopefully its a legible PR otherwise.

Changes

  • Add UI to frontend to choose brushless hardware
  • Add new number minPenPosition to Plan, which I think is different for each kind of hardware
    • This might be part of fixing the pen up/down not moving the full range on the brushless motor
  • Re-adding the 'last move' pen up position logic in Planning.plan
    • I think this was removed during another rebase, not sure how intentional that was
  • Broadcast the com path used in the server to all websockets (was hardcoded to /dev/XXX before)

Refactoring

  • Move the --hardware cli option to be global, so it can be used in saxi plot.
  • Switch brushless from a boolean to hardware enum
    • This just made it easier for me to refactor
  • Add hardware to PlanOptions, so that Planning.plan works
    • There may be a way to remove hardware in the future with further refactoring
  • A lot of small typescript and eslint cleanups, mostly harmless

Copy link
Collaborator Author

@jedahan jedahan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will continue giving file-by-file context soon

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Main change is making the type of --hardware an enum. Everything else is just style.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making --hardware an enum

@jedahan
Copy link
Collaborator Author

jedahan commented Sep 20, 2023

@alexrudd2 mind rebasing your brushless branch on top of your main to include the esbuild stuff?

@alexrudd2
Copy link
Owner

@alexrudd2 mind rebasing your brushless branch on top of your main to include the esbuild stuff?

Ok, done. Not tested, though :)

@alexrudd2
Copy link
Owner

I'm OK with all your changes generally, but this branch has gotten really unwieldy.

Please check out #87, which clears up some of the merge conflicts.

Also, it would be easier to land a lot of the stuff (like the formatting, or splitting the ts-config) separately.

@jedahan
Copy link
Collaborator Author

jedahan commented Sep 27, 2023

Apologies for all the extra work I made, I am going to close this PR and work off the other one.

@jedahan jedahan closed this Sep 27, 2023
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