scriptorron / indi_pylibcamera

INDI libcamera driver made in Python
MIT License
11 stars 5 forks source link

IMX290 crashes or hangs after first exposure #53

Closed anjok closed 7 months ago

anjok commented 7 months ago

Not all the time, but most. This here seems to fix it:

diff --git a/src/indi_pylibcamera/CameraControl.py b/src/indi_pylibcamera/CameraControl.py
index 50497ca..7bc8813 100755
--- a/src/indi_pylibcamera/CameraControl.py
+++ b/src/indi_pylibcamera/CameraControl.py
@@ -150,7 +150,7 @@ class CameraSettings:
             self.is_ReconfigurationNeeded(NewCameraSettings)
             or (self.camera_controls != NewCameraSettings.camera_controls)
          )
-        return is_RestartNeeded
+        return is_RestartNeeded or True

     def is_ReconfigurationNeeded(self, NewCameraSettings):
         """would using NewCameraSettings need a camera reconfiguration?
@@ -161,7 +161,7 @@ class CameraSettings:
             or (self.ProcSize != NewCameraSettings.ProcSize)
             or (self.RawMode != NewCameraSettings.RawMode)
         )
-        return is_ReconfigurationNeeded
+        return is_ReconfigurationNeeded or True

     def __str__(self):
         return f'CameraSettings: FastExposure={self.DoFastExposure}, DoRaw={self.DoRaw}, ProcSize={self.ProcSize}, ` +\
~
anjok commented 7 months ago

Note that this only works with stellarmate when you do the song and dance from here: https://indilib.org/forum/ccds-dslrs/12177-indi-libcamera-driver.html?start=216#97003

Otherwise. you can't even install PiCamera2

anjok commented 7 months ago

In case it's important, here's my /boot/config.txt

camera_auto_detect=0
dtoverlay=imx290, clock-frequency=37125000
scriptorron commented 7 months ago

Hi Anjok,

thank you for reporting and providing a workaround.

The development of libcamera and its Python binding picamera2 is very dynamic. During the last months I saw that features got not only added but also changed in an incompatible way. That makes it very difficult for me to make the indi_pylibcamera driver compatible to a wide range of libcamera/picamera2 versions. I try to make it work with the latest versions but often that is at the expense of compatibility to older versions. I am sorry that Stellarmate and Astroberry have old versions of libcamera and picamera2 and indi_pylibcamera is not compatible. Hopefully these distributions will get updates soon. In the Wiki is a description how I setup my Raspberry Pi based on the standard Bullseye OS.

Back to the issue you reported. According to the picamera2 documentation it is allowed to capture multiple frames between starting and stopping the camera. That works with HQ and V1 camera but not with the IMX290. With your workaround you force a camera stop and re-start before every frame capture. It is save doing this. The only concern I have is that it may add delay. I remember that the stop start functions block execution for some seconds (depending on exposure time?). That would make the "Fast Exposure" feature inefficient. I need to do some tests.

I would propose to add a configuration switch in the INI file which allows to force the stop/restart (as in your workaround). That gives the highest flexibility. If you agree it will become part of the next release.

Regards, Ronald

anjok commented 7 months ago

Hi, thanks for your reply and the great work you did with this driver!

I did some work on the indi libcamera driver and it really wasn't fun... given this mix of astroberry not being on bullseye, stellarmate having this really outdated versions etc it's a train wreck waiting to happen :)

You can do this any way you think is best, but this is a simpler version of the one above. If self.CamProps['Model'] was present at the time of the call it would be easiest, but it's apparently loaded later and I didn't know if I can move it. If you can do that, you can remove the or True.

I guess that this is a global problem and not just my setup, so I'd just add this to the code and not fiddle with settings.

This seems to work fine for both the imx290 and imx519 I have (and at least for me it's needed for both of them).

index 50497ca..b4434cd 100755
--- a/src/indi_pylibcamera/CameraControl.py
+++ b/src/indi_pylibcamera/CameraControl.py
@@ -160,8 +160,10 @@ class CameraSettings:
             or (self.DoRaw != NewCameraSettings.DoRaw)
             or (self.ProcSize != NewCameraSettings.ProcSize)
             or (self.RawMode != NewCameraSettings.RawMode)
+            # this apparently isn't loaded here...
+            # or (self.CamProps["Model"] in ['imx290','imx519'])
         )
-        return is_ReconfigurationNeeded
+        return is_ReconfigurationNeeded or True

     def __str__(self):
         return f'CameraSettings: FastExposure={self.DoFastExposure}, DoRaw={self.DoRaw}, ProcSize={self.ProcSize}, ` +\
anjok commented 7 months ago

OK, so at least for the imx519 you can move the or True to is_RestartNeeded. Dunno what difference this makes. I just checked a few pics with differing exposure times.

index 50497ca..1de45db 100755
--- a/src/indi_pylibcamera/CameraControl.py
+++ b/src/indi_pylibcamera/CameraControl.py
@@ -150,7 +150,7 @@ class CameraSettings:
             self.is_ReconfigurationNeeded(NewCameraSettings)
             or (self.camera_controls != NewCameraSettings.camera_controls)
          )
-        return is_RestartNeeded
+        return is_RestartNeeded or True

     def is_ReconfigurationNeeded(self, NewCameraSettings):
         """would using NewCameraSettings need a camera reconfiguration?
anjok commented 7 months ago

I would propose to add a configuration switch in the INI file which allows to force the stop/restart (as in your workaround). That gives the highest flexibility. If you agree it will become part of the next release.

The problem with that is that I have multiple cameras. I don't know how this would work with the INI file? I apparently need to change /boot/config.txt and reboot when I use a new one, but I'd prefer if I didn't need to change this in multiple places. I would think checking the names of known non working cams would be more robust... we just need to get the cam props up front (or have it return True on the first run)

scriptorron commented 7 months ago

I understand. What about an INI switch "ForceRestart" with 3 states:

I can make the "auto" the default when the switch is not in the INI file. In that way you do not need to change something on your INI file and the driver detects your cameras automatically. When someone comes with a different non-working camera I have a quick workaround with "yes". Once the kernel driver for your cameras gets fixed you can test it with "no".

I already do some special handling for different camera chips. It would be only a relatively small code change.

scriptorron commented 7 months ago

Implemented but not tested in branch 53-imx290-crashes-or-hangs-after-first-exposure. It is even not tested for syntax errors. If family allows I will test with my cameras tomorrow.

anjok commented 7 months ago

Tried with the imx290, seems to work fine. No hangs. what do you think about changing the logging to actually end up in kstars? It’s a bit inconvenient to look in /tmp…

scriptorron commented 7 months ago

Thank you for testing. I also tested with HQ camera and did not found any issues (neither with the auto nor with the yes setting). Unfortunately I do not have a setup with V1 camera anymore. But I believe no one will use V1 for astrophotography - it is too bad.

Forwarding the logging to kstars is a good idea. But before I do this I will need to reduce the amount of messages. I will work on it.