NARUTOfzr / Neptune_3

source code
71 stars 26 forks source link

consider a clean Marlin fork #22

Open Krifto opened 1 year ago

Krifto commented 1 year ago

Could you please let us know which exact github revision (or tag) of Marlin is this forked from?

And please consider creating a clean fork from that revision, then applying any customizations on top of that, instead of copy-pasting the entirety of Marlin into versioned subfolders.

Some benefits that come to mind:

Unfortunately, the current repository structure really only fosters a wild growth of forks that apply only to a specific version (i.e. folder) of the firmware.

Related items: #6, #15 and #18

mlee12382 commented 1 year ago

There is no on-board uart, and enabling it it near impossible for most people. I don't disagree with you on the rest however. Unfortunately so many of the core source files have been modified that pulling from the main marlin repo for updates is impossible. This appears to be a heavily modified version of 2.0.9.3 If you want to see what happens with a clean version of marlin where only the new board has been added to the source files look here: https://github.com/mlee12382/Elegoo-Neptune-Marlin/tree/elegoo-neptune-3 it will compile if you bypass the relevant sanitycheck entry for the false positive on the FAN_PIN error. It doesn't interact nicely with the screen firmware though.

Krifto commented 1 year ago

Sorry, instead of UART I was referring to the RX/TX pins that were enabled in this fork: https://github.com/Piets/Neptune_3/commit/42a78757cc19da64b5c44fc6d0170b694ad99b3e

It could still be useful to apply the bulk of all those heavy changes on top of that particular Marlin base version, and then start tracking changes from there on out, with release tags instead of subfolders. That way we can at least see where the original Marlin ends and the Neptune 3 mods start.

Good point on the screen firmware, does that have an upstream version as well?

mlee12382 commented 1 year ago

Ahh ok I wasn't aware of that mod but that makes sense, I was thinking of doing something similar on the N2 board for neopixels before I killed a driver and decided to upgrade the board. It would definitely be nice to have a history of all the changes that have been made to the source files, that's one of the biggest frustrations. As for the screen I have a guy that's been helping me test my clean firmware and was going off the assumption that since the elegoo screen is a clone of the mks h43 that the mks screen firmware would work, unfortunately the elegoo screen uses a T5L0 chip and the mks uses a T5L1 and the firmware are not compatible so I'm not aware of an upstream screen firmware that's available. The mks firmware locks up the screen, my friend has a programmer coming and we think we have a way to unlock it but that's not a viable option for the general public so I immediately pulled the firmware that I had on my git before anyone else messed up their screens lol

NARUTOfzr commented 1 year ago

Could you please let us know which exact github revision (or tag) of Marlin is this forked from?

And please consider creating a clean fork from that revision, then applying any customizations on top of that, instead of copy-pasting the entirety of Marlin into versioned subfolders.

Some benefits that come to mind:

  • it becomes possible to observe the on-going evolution of this firmware in the git log
  • tests can be run the same way as in the Marlin repo
  • upstream changes/fixes in Marlin could be pulled into this repository much more easily
  • structured pull requests can be provided that work the same way as in the Marlin repo
  • popular forks could be pulled back into this repository as branches (e.g. the modifications for dual-z steppers, or enabling the on-board ~UART~ RX/TX-pins)
  • generally the community could help out much more easily, which ultimately can help the popularity of the printer

Unfortunately, the current repository structure really only fosters a wild growth of forks that apply only to a specific version (i.e. folder) of the firmware.

@Krifto First of all, thanks for Michael's answer! Also thanks for your advice, I am trying to create branches to pull requests.

benjaminbrumbaugh commented 1 year ago

Isn’t UART needed for most of the fancy new Klipper features? What would it take?

On Oct 5, 2022, at 5:13 AM, Michael @.***> wrote:

 There is no on-board uart, and enabling it it near impossible for most people. I don't disagree with you on the rest however. Unfortunately so many of the core source files have been modified that pulling from the main marlin repo for updates is impossible. This appears to be a heavily modified version of 2.0.9.3 If you want to see what happens with a clean version of marlin where only the new board has been added to the source files look here: https://github.com/mlee12382/Elegoo-Neptune-Marlin/tree/elegoo-neptune-3 it will compile if you bypass the relevant sanitycheck entry for the false positive on the FAN_PIN error. It doesn't interact nicely with the screen firmware though.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.

mlee12382 commented 1 year ago

Isn’t UART needed for most of the fancy new Klipper features?

Yes and no, you can still use klipper without uart for the drivers, you just have to manually set vref and there's no stealthchop or sensorless homing capabilities and things like that that require drivers in uart mode.

What would it take?

Very delicate soldering skills to connect a jumper to the TINY driver chip pin, knowing which pin and getting a wire attached to it without also connecting to other pins on the chip that you don't want to mess with is difficult at best. If you can do that then you have to find unused pins on the board to attach them to and assign them to uart control for each axis.

Krifto commented 1 year ago

I went ahead and created a new fork of Marlin, and applied all the changes from this repository to it, to give an idea of how this could look like:

https://github.com/Krifto/Marlin-Neptune3

Basically, you are looking at a pretty standard Marlin repository with all Neptune3 changes on top. I took the Neptune3 README.md as the default. And I have NOT added any binaries/firmwares to that repository.

The right way to create binary releases now would be through the "releases" functionality of github, instead of commiting binaries to the repository.

@NARUTOfzr what is your build process? Just platformio run and then rename the genreated firmware.bin? And how are the binaries for the LCD screen firmware created?

We could probably set up pipelines to automate all of that, including automated tests...

mlee12382 commented 1 year ago

Just platformio run and then rename the genreated firmware.bin?

It automatically renames the file due to the instructions in the platformio board instructions here: https://github.com/NARUTOfzr/Neptune_3/blob/main/Source%20Code/ZNP_F401_Marlin-Beta/ini/stm32f4.ini

Krifto commented 1 year ago

This is my output for pio run in VSCode (also running it on your fork, @mlee12382). The filename is not the same as in the released firmware folder, just a plain firmware.bin:

Advanced Memory Usage is available via "PlatformIO Home > Project Inspect"
RAM:   [=         ]  14.0% (used 9192 bytes from 65536 bytes)
Flash: [======    ]  56.5% (used 148088 bytes from 262144 bytes)
Building .pio\build\MKS_E3_V2\firmware.bin
rename_target([".pio\build\MKS_E3_V2\firmware.bin"], [".pio\build\MKS_E3_V2\firmware.elf"])
=================================================================================================================================== [SUCCESS] Took 52.40 seconds ===================================================================================================================================
Environment    Status    Duration
-------------  --------  ------------
MKS_E3_V2      SUCCESS   00:00:52.396

On another note: Could you maybe add an Issues tab to your fork of this repo? Probably better to have discussions your advanced firmware (which sounds awesome) a bit separate - what do you think?

mlee12382 commented 1 year ago

LOL 🤦 I thought issues were on by default, didn't even realize I didn't have them available. Fixed it. I wonder if you grabbed an older version of the code? I think it should be set for renaming on the current version but you can update the stm32f4.ini in the ini folder to change it. Under env: mks_e3_v2 there should be a line that says: board_build.rename = ZNP_ROBIN_NANO.bin

Krifto commented 1 year ago

Edit: I had referenced the ini file here but overlooked that the entry was there. But for some reason my platformio does not mention the renaming in the output, when it indeed does the renaming, or at least the log line rename_target([".pio\build\MKS_E3_V2\firmware.bin"], [".pio\build\MKS_E3_V2\firmware.elf"]) does not say what it is renaming it to...

On the overall idea of cleaning up the repo structure I'd like to keep discussing over in your fork's issue tracker.

mlee12382 commented 1 year ago

Yeah I've noticed that also, there's no mention in the output log but when you go to the folder it has the correct name.

David-McKenna commented 1 year ago

This appears to be a heavily modified version of 2.0.9.3

e0f75d4f069b80ec78ee911377861aa5e77f2a14 seems to be the specific commit for anyone interested in trying to get things merged as easily as possible, checking a few of the linked repos here they're modified against an earlier commit, so some of the changes were made by Marlin, not by Elegoo. I've given it a go to merge the "beta" firmware folder in this repo without any further changes (apart from ignoring all the added whitespace) against that commit to the bugfix-2.0.x branch and the latest bugfix-2.1.x branch in the links below.

Apart from confirming that they compile, I have no other guarantees with respect their functionality as I can't test them right now (machine out of action for the moment), but will be giving the v2.1.x version a go in a couple of days when I get replacement parts in.

https://github.com/David-McKenna/Marlin/tree/neptune3-1.0.beta-bugfix-2.0.x https://github.com/David-McKenna/Marlin/tree/neptune3-bugfix-2.1.x

Krifto commented 1 year ago

e0f75d4f069b80ec78ee911377861aa5e77f2a14 seems to be the specific commit for anyone interested in trying to get things merged as easily as possible

Thanks a lot! To be consistent I rebased the branch in my repo on this commit, indeed this is a much smaller diff than the one from 2.0.9.3: https://github.com/Krifto/Marlin-Neptune3/commit/37ee12f30ae5bf800c2a3dbde14769ef95b58893

Awesome work on your merge to the bugfix-2.1.x! Please keep us updated - a working N3 firmware based on the most up-to-date Marlin sounds fantastic!

David-McKenna commented 1 year ago

Ran into a bunch of random issues using the head of bugfix-2.1.1 as a base, so I reverted to the 2.1.1 release and have tried to build things up from there. The branch linked below seems to be running relatively smoothly (had no issues with 2 10+ hours prints over the last few days) if people want to give it a shot, though there's a few issues I haven't got a solution for as of yet:

I was also trying to enable linear advance (not committed), but despite my efforts I can never get the Marlin-generated calibration print to extrude plastic beyond the first half of the purge line (despite the extruder rollers moving fine, setting K=0.1 during a print doesn't appear to cause any issues). I swear it worked once, but I can't re-find the configuration I used to compile that image unfortunately.

https://github.com/David-McKenna/Marlin/tree/neptune3_merge_2.1.1