Desuuuu / Marlin

Optimized firmware for RepRap 3D printers based on the Arduino platform. Modified with a new DWIN T5UID1 touchscreen implementation.
GNU General Public License v3.0
31 stars 10 forks source link

Minimal DGUS Reloaded config files #7

Closed mrv96 closed 3 years ago

mrv96 commented 3 years ago

I saw that your two configuration files are the stock ones of official Marlin repo. I know that you have a repo with custom configs files, but please consider to modify them also in this repo with the minimal changes that allow to use your additional DGUS Reloaded code.

Desuuuu commented 3 years ago

I don't really see a point in doing this to be honest. It makes more sense to me to have a 'blank' configuration here and separate configurations for each printer.

Anything but DGUS_LCD_UI_RELOADED here is printer-specific.

mrv96 commented 3 years ago

When i downloaded your repo to install your firmware on my printer, i performed a number of annoying steps due to your static_asserts.

It would be easier form me to start from a minimal working configuration to be modified for my printer.

IMHO there is the necessity of a clean but working config from which to start.

Desuuuu commented 3 years ago

The example configuration files in the other repository (linked near the top of the README) do provide a working base for pretty much all the printers this firmware can run on.

Also, you can't really have a working configuration without knowing which hardware you're running on (which is why example configurations are specific to each printer).

mrv96 commented 3 years ago

There aren't only Creality 3D printers that can run your code. Any printer with a mega2560 based mainboard (and there are a lot of different boards available) and a DWIN display can run your code. For example i'm using it on a Longer LK4 Pro

Of course you are right for what concern the hardware, but the idea is to have a minimal example configuration which can be compiled. This is not a my idea, it is taken from Marlin. Marlin supports many boards and many options, but by default it is set to be compiled for an atmega2560 based board.

My idea was to have this minimal Marlin config slightly modified to highlights your additional code. Of corse the final choice is yours.

P.S. your work is very good! It seems that works good. Please consider to merge it in main Marlin Repo

Desuuuu commented 3 years ago

Considering the screen firmware was built around some pretty specific hardware, I feel like the example configuration for the CR-10S Pro is pretty much the minimal configuration required (as you've noticed, there are quite a few requirements in terms of hardware).

Changing the configuration here doesn't really make much sense to me. With that said, if you feel like submitting a PR on the configurations repository for your printer, I'll gladly accept it!

Regarding Marlin, it already has a DGUS implementation integrated so I'm not too sure this would be accepted there, but I'll keep that in mind, thanks!

mrv96 commented 3 years ago

Ok. Thank you for your feedback. Maybe I'll make a PR in your config repo.

Your work is extremely better than the one in official Marlin, moreover the creator of that code is out of the picture and the official DWIN code is no longer mantained.

This is the last message of the person who wrote that code: https://github.com/MarlinFirmware/Marlin/issues/12096#issuecomment-642507556