mryel00 / spyglass

A simple mjpeg server for Picamera2
GNU General Public License v3.0
68 stars 15 forks source link

fix(install): fix missing directory during install #73

Closed inferno0230 closed 4 months ago

roamingthings commented 5 months ago

Thank you for your contribution. Could you please add some context to the pull request.

inferno0230 commented 5 months ago

Thank you for your contribution. Could you please add some context to the pull request.

for commit fix(Makefile): Ensure printer_data directory exists before installation

fixes:

root@rpi:/home/inferno0230/spyglass# make install

Copying systemd service file ...

Copying Spyglass launch script ...

Copying basic configuration file ...

cp: cannot create regular file '/home/root/printer_data/config': No such file or directory

and for fix(Makefile): Properly set PWD variable, i mentioned in commit message

mryel00 commented 4 months ago

@inferno0230 can you give an example on when the PWD variable might not be set? As this software is only targeting Raspberry Pis/RaspberryPiOS I don't think setting the variable is needed.

inferno0230 commented 4 months ago

@inferno0230 can you give an example on when the PWD variable might not be set? As this software is only targeting Raspberry Pis/RaspberryPiOS I don't think setting the variable is needed.

inferno0230 in ~/spyglass on main λ sudo make install

Copying systemd service file ...
cp: cannot stat '/resources/spyglass.service': No such file or directory
make: *** [Makefile:23: install] Error 1
inferno0230 in ~/spyglass on main λ neofetch
       _,met$$$$$gg.          inferno0230@rpi5 
    ,g$$$$$$$$$$$$$$$P.       ---------------- 
  ,g$$P"     """Y$$.".        OS: Debian GNU/Linux 12 (bookworm) aarch64 
 ,$$P'              `$$$.     Host: Raspberry Pi 5 Model B Rev 1.0 
',$$P       ,ggs.     `$$b:   Kernel: 6.6.28-v8-16k+ 
`d$$'     ,$P"'   .    $$$    Uptime: 1 day, 4 hours, 7 mins 
 $$P      d$'     ,    $$P    Packages: 2024 (dpkg) 
 $$:      $$.   -    ,d$$'    Shell: zsh 5.9 
 $$;      Y$b._   _,d$P'      Terminal: /dev/pts/0 
 Y$$.    `.`"Y$$$$P"'         CPU: (4) @ 2.800GHz 
 `$$b      "-.__              Memory: 840MiB / 8052MiB 
  `Y$$
   `Y$$.                                              
     `$$b.                                            
       `Y$$b.
          `"Y$b._
              `"""

but now that i think about it, sudo isn't really needed, it was just my habit of using sudo with make installs

also, is the spyglass script finding the config at wrong location

https://github.com/roamingthings/spyglass/blob/main/Makefile#L14..#L15 whereas

inferno0230 in ~/spyglass on main λ cat ./scripts/spyglass | grep CFG | head -n 1

SPYGLASS_CFG="${HOME}/printer_data/config/spyglass.conf"
mryel00 commented 4 months ago

but now that i think about it, sudo isn't really needed, it was just my habit of using sudo with make installs

So ig this won't be needed then. But I'm wondering if that installation of yours is a plain Debian or RaspberryPiOS. If a plain Debian, does the CSI cam work?

also, is the spyglass script finding the config at wrong location

That's again only the case for sudo, isn't it? As that would populate the user wrong and then breaks everything.

inferno0230 commented 4 months ago

So ig this won't be needed then

yeah, not needed

But I'm wondering if that installation of yours is a plain Debian or RaspberryPiOS

Its rpios

That's again only the case for sudo, isn't it? As that would populate the user wrong and then breaks everything.

nah, even without sudo it will break config copy cause what it's doing is

cp -f "./spyglass/resources/spyglass.conf /home/$(USER)/printer_data/config
or
cp spyglass.conf config

this can be fixed by

diff --git a/Makefile b/Makefile
index 29ab294..4ca56a4 100644
--- a/Makefile
+++ b/Makefile
@@ -18,7 +18,7 @@ all:
        $(MAKE) help

 install: ## Install Spyglass as service
-       @mkdir -p $(PRINTER_DATA_PATH)
+       @mkdir -p $(PRINTER_DATA_PATH)/config
        @printf "\nCopying systemd service file ...\n"
        @sudo cp -f "${PWD}/resources/spyglass.service" $(SYSTEMD)
        @sudo sed -i "s/%USER%/$(USER)/g" $(SYSTEMD)/spyglass.service
mryel00 commented 4 months ago

Oh you mean it's creating the wrong directory in your PR. I totally missed that ^^

mryel00 commented 4 months ago

this can be fixed by

diff --git a/Makefile b/Makefile
index 29ab294..4ca56a4 100644
--- a/Makefile
+++ b/Makefile
@@ -18,7 +18,7 @@ all:
        $(MAKE) help

 install: ## Install Spyglass as service
-       @mkdir -p $(PRINTER_DATA_PATH)
+       @mkdir -p $(PRINTER_DATA_PATH)/config
        @printf "\nCopying systemd service file ...\n"
        @sudo cp -f "${PWD}/resources/spyglass.service" $(SYSTEMD)
        @sudo sed -i "s/%USER%/$(USER)/g" $(SYSTEMD)/spyglass.service

The better solution would be to just use the CONF_PATH instead of PRINTER_DATA_PATH

inferno0230 commented 4 months ago

this can be fixed by

diff --git a/Makefile b/Makefile
index 29ab294..4ca56a4 100644
--- a/Makefile
+++ b/Makefile
@@ -18,7 +18,7 @@ all:
        $(MAKE) help

 install: ## Install Spyglass as service
-       @mkdir -p $(PRINTER_DATA_PATH)
+       @mkdir -p $(PRINTER_DATA_PATH)/config
        @printf "\nCopying systemd service file ...\n"
        @sudo cp -f "${PWD}/resources/spyglass.service" $(SYSTEMD)
        @sudo sed -i "s/%USER%/$(USER)/g" $(SYSTEMD)/spyglass.service

The better solution would be to just use the CONF_PATH instead of PRINTER_DATA_PATH

mkdir -p $(CONF_PATH) ? hmm, that would fix both issues (or should i say the issue from before my pr and the issue created by my pr lol)

inferno0230 commented 4 months ago
inferno0230 in ~/spyglass on main λ make install

Copying systemd service file ...

Copying Spyglass launch script ...

Copying basic configuration file ...

Populate new service file ... 

Enable Spyglass service ... 

To be sure, everything is setup please reboot ...
Thanks for choosing Spyglass ...
inferno0230 in ~/spyglass on main λ ls ~/print*/config
spyglass.conf

inferno0230 in ~/spyglass on main λ ..
inferno0230 in ~ λ spyglass
INFO: Python interpreter Version Python 3.11.2 found ... [OK]
INFO: Configuration file found in /home/inferno0230/printer_data/config/spyglass.conf
INFO: Print Configfile: '/home/inferno0230/printer_data/config/spyglass.conf'
                NO_PROXY="true"
                HTTP_PORT="8080"
                RESOLUTION="640x480"
                FPS="15"
                STREAM_URL="/stream"
                SNAPSHOT_URL="/snapshot"
                AUTO_FOCUS="continuous"
                FOCAL_DIST="0.0"
                AF_SPEED="normal"
                ORIENTATION_EXIF="h"
INFO:root:Spyglass 0.13.0
[29:21:08.850153731] [351497]  INFO Camera camera_manager.cpp:284 libcamera v0.2.0+120-eb00c13d
...

seems fine now