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

Add Linux support to the Makefile #64

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sodaplayer
Copy link

I've added detection for Linux to the Makefile which will use a Linux container to build and run the bintosyx command. Maybe in the future, the Dockerfile can handle the main build as well.

@mat1jaczyyy mat1jaczyyy self-assigned this Sep 16, 2021
Copy link
Owner

@mat1jaczyyy mat1jaczyyy left a comment

Choose a reason for hiding this comment

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

Generally approving, but I need to test for regressions before ready to merge.

-v $(shell pwd)/$(BUILDDIR):/tmp/utils/$(BUILDDIR) \
bintosyxbuilder \
./tools/out/bintosyx /pro 000 $(BIN) $(SYX)
else
Copy link
Owner

Choose a reason for hiding this comment

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

Interested in your thoughts on having macOS build the bintosyx tool via Docker from the other repo as well. The only counter-argument I can think of is that macOS machines don't come with gcc preinstalled (but, anyone tinkering with this on a Mac is likely to have gcc installed anyway).

For Windows I'd avoid using Docker and keep the currently prebuilt EXE because you have to either make VS a requirement or deal with the mess of using gcc on Windows, both of which I'd prefer to avoid in a firmware project.

Copy link
Author

Choose a reason for hiding this comment

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

I could go either way for Mac, assuming the already provided tools/bintosyx binary has been working. Maybe in another branch, I can experiment with it and let you know what I think.

README.md Outdated

#### Flashing

The MIDI SysEx utility, `amidi`, comes preinstalled on most Linux distributions
Copy link
Owner

Choose a reason for hiding this comment

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

The recommended way to flash on all platforms is through https://fw.mat1jaczyyy.com, though I suppose a short mention of amidi can be added to the Installation section near the beginning of the readme instead of the full blown explanation.

Copy link
Author

Choose a reason for hiding this comment

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

Sounds good! Cleared out the full explanation from the README and added amidi to the list at the top.

* Remove full amidi explanation
* Add amidi to list of SysEx-capable programs
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.

None yet

3 participants