ArduCAM / Arducam_tof_camera

51 stars 20 forks source link

arducamCameraClose typing mistake? #78

Open ZacJW opened 3 months ago

ZacJW commented 3 months ago

I'm working on writing bindings for ArducamDepthCamera2c in another language, and I'm refering to ArducamDepthCamera.h from arducam-tof-sdk-dev 0.1.5 from the PPA and preview_depth.c from this repo. In doing so I think I've stumbled across a mistake in one of these files, but I can't tell which.

Most of the lifecycle functions on ArducamDepthCamera just take the value directly:

extern Status arducamCameraOpen(ArducamDepthCamera camera, ArducamConnection conn, int index);

extern Status arducamCameraStart(ArducamDepthCamera camera, ArducamFrameType type);

extern Status arducamCameraStop(ArducamDepthCamera camera);

but arducamCameraClose takes a pointer to it:

extern Status arducamCameraClose(ArducamDepthCamera* camera);

At first I thought this was intentional (like it was an in-out pointer) but after looking at preview_depth.c I'm not so sure. Here's the part of interest:

int main()
{
    ArducamDepthCamera tof = createArducamDepthCamera();
    ArducamFrameBuffer frame;
    if (arducamCameraOpen(tof, CSI, 0)) {
        printf("Failed to open camera\n");
        return -1;
    }

    if (arducamCameraStart(tof, DEPTH_FRAME)) {
        printf("Failed to start camera\n");
        return -1;
    }
    // ...

   if (arducamCameraStop(tof)) {
        return -1;
    }

    if (arducamCameraClose(tof)) {
        return -1;
    }
    return 0;
}

arducamCameraOpen, arducamCameraStart, arducamCameraStop, and curcially arducamCameraClose are called in the exact same way. The only reason this isn't a compiler error is because ArducamDepthCamera is just an alias for void* and C implicitly coerces the void* tof into the void** required by arducamCameraClose, but surely that can't be sound. Since there's an infinite for (;;) loop above with no break the lines after are unreachable so no-one will have encoutered whatever happens at runtime when arducamCameraClose is called like this. I only noticed this because the language I'm writing the bindings in (Rust) doesn't perform this implict coercion like C does so I got a type error.

If arducamCameraClose's signature is correct can the example be corrected, otherwise can the signature be corrected?