search

mendhak / waveshare-epaper-display

At-a-glance dashboard for Raspberry Pi with a Waveshare ePaper 7.5 Inch HAT. Date/Time, Weather, Alerts, Google/Outlook Calendar
https://code.mendhak.com/raspberrypi-epaper-dashboard/
438 stars 65 forks source link

Waveshare's code should probably not be directly copied into the project #20

Closed lifeofbrian closed 3 years ago

lifeofbrian commented 3 years ago

It appears as though the display path is from this project's directory: https://github.com/waveshare/e-Paper/tree/a7aba1d7466a62d56ef805491467b5d7a9264277/RaspberryPi_JetsonNano/c

I'm going to attempt to use this waveshare repo as a submodule to address the concern

mendhak commented 3 years ago

Ah the display directory became unnecessary after I started using a local display.py. Admittedly I've avoided submodules as they don't play well with CI/CD, but for this project it's probably more convenient for the user to just get everything in one go, makes sense.

I haven't looked properly at the PR yet but will soon, but yeah I see at first glance it's a boatload of overdue cleanup which is good.

lifeofbrian commented 3 years ago

Agreed - the CI/CD concern with submodules went through my mind as well, but that seems to be a lower priority, for the time being for this project.

Thanks for looking at the PR.

mendhak commented 3 years ago

PR merged. It's simpler now for the user, thanks. I made some minor changes, added the --recursive flag in the README instructions too. Also added ignore=dirty in the .gitmodule so that it doesn't show up as changes in git status, in case it confuses anyone (like me).