Skip to content

Conversation

@onekiloparsec
Copy link

@onekiloparsec onekiloparsec commented Sep 22, 2025

Summary

This PR solves #23, and introduces a new accessor ImageArrayRaw to the Camera class.

Description

As explained in #23, there is a tiny issue with the tcode inferred when reading the image array from the camera. The types l and L are not fully portable, since it doesn't work on macOS. On macOS, the C type "long" is 8 bytes, while on Linux and Windows it’s usually 4 bytes. Hence, it is recommended to change these to i and I respectively, which are fully portable on every platform. I couldn't find an explicit Python ref to paste here unfortunately.

While doing tests to validate the solution, it became obvious the low-level routine would benefit from a small refactoring, to be split in multiple smaller methods with separated concerns. This is what this PR also introduces.

The original accessor def ImageArray(self) has been completely preserved. But in the case of binary data, the building of nested lists isn't always necessary, nor really desirable. Hence, thanks to the refactoring, it became possible to expose right-away the array.array (which is anyway built) via a new accesor def ImageArrayRaw(self).

Open questions / notes

  • I thought it would be simpler to make one PR with the l/L->i/I changes and the reafctoring, but I'm open to split the PR if you prefer.
  • If you prefer to name the accessor differently, I'd be happy to change.
  • As far as I can tell, all tests pass (but I would consider an additional section of tests with the simulators quite great - subject to another PR probably).
  • I tried to respect as much as possible to the original formatting, although it is rather non-standard. Please, let me know if I need to correct anything.

…e array.

Signed-off-by: Cédric Foellmi <cedric@onekiloparsec.dev>
@onekiloparsec
Copy link
Author

onekiloparsec commented Oct 20, 2025

Dear @BobDenny it seems you are the author of this fantastic lib, and I am pretty sure you may be busy with other tasks. But would you mind having a quick look at this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant