alliedvision / VimbaPython

Old Allied Vision Vimba Python API. The successor to this API is VmbPy
BSD 2-Clause "Simplified" License
93 stars 40 forks source link

vimba.Camera class method to set a feature to a desired value #132

Open CrocodileDandy opened 2 years ago

CrocodileDandy commented 2 years ago

It seems to me that the vimba.Camera class does not possess a simple method to set a feature to a desired value. What is the reason for this? Is it because the Python wrapper only wraps the C API which does not possess such functionality?

Such a method could be implemented in the vimba.Camera class in the following way:

@TraceEnable()
@RaiseIfOutsideContext()
@RuntimeTypeCheckEnable()
def set_feature(self, camera: vimba.Camera, feature_name: str, value, verbose: bool=False):
        with vimba.Vimba.get_instance():
            with camera:
                feature = camera.get_feature_by_name(feature_name)
                feature.set(value)
                if verbose:  ## Adding some verbose for clarity
                    print(f"Feature {feature.get_name()} set to: {feature.get()} {feature.get_unit()}")

Is there any interest to implement such a method?

CrocodileDandy commented 2 years ago

I realized one can also do vimba.Camera.feature_name.set(value) although I am not too much of a fan of this. Not sure what is the best / safest. Any recommendation?

Teresa-AlliedVision commented 2 years ago

Hello CrocodileDandy, some of the background with the way .set(value) works here, is that it is close to how features are set in the other language APIs (you get the camera, then a feature from the camera by name and then set the value in the feature). The Python API lets you takes some shortcuts generally and has some convenience functions, but it is conceptionally a prototype API. For better performance it is usually recommended to switch to the others for the final products, so it is in our best interest to keep the handling somewhat similar.

The values that can be set, differ a lot between features and cameras (and sometimes change due to other features being set), because the value needs to be set in the physical camera to something that the firmware accepts. To set features it is therefore necessary to keep the feature type (int, enum, string, float), enum options, range and increment in mind. It should be possible to have some convenience around that, but the way I see it, it is more robust for the application if the programmer implements the needed functions themselves, depending on the feature, application and camera. But I also mainly use the C and C++ API, so I might be biased here.

Anyways, I hope that clears up why it might be set up the way it is. @NiklasKroeger-AlliedVision can take the suggestion and propose a change.

Cheers, Teresa

CrocodileDandy commented 2 years ago

Hello @Teresa-AlliedVision,

It should be possible to have some convenience around that, but the way I see it, it is more robust for the application if the programmer implements the needed functions themselves, depending on the feature, application and camera.

Yes. I believe you are right and this always should be the way. To be honest, I like the process that consists in getting the camera, getting the feature by name from the camera and then setting it. I think it's pretty safe as it takes no Python shortcuts (such as stealthily using vimba.Camera.__set_attr_() to set features without guarantee or them being set properly or simply destroyed and replaced by an attribute) and fully relies on the C API.

I just wondered if my vimba.Camera.set_feature() implementation suggestion was generic and safe enough to be standard. But then again, if it's not mirroring a functionality of the C API, it may not belong to the Python wrapper.

The reason I wanted to implement this at the vimba.Camera level is because I believe such a functionality belongs to a Camera object and I don't want to subclass vimba.Camera to implement this or implement it somewhere else as I'm doing now. It creates some undesired complexity IMHO.

Teresa-AlliedVision commented 2 years ago

Hello CrocodileDandy, I think with some modifications your function can work, you just need to adapt it for the different cases.

  1. Read out the Feature Type from the feature
  2. Implement different cases for Enum, Int, Float and Command features
  3. For Float, Int, Enum: Check if they are writable (there is feature information "writable" and "readable", that are binary 0 or 1)
  4. If they are writable: Read out the range and increment for int and float features and implement a nearest value function
  5. If it is an enum feature, read out the different values that the feature can have and choose the closest to the input or let the user chose (an Enum Fetaure might not accept "off" when it wants "Off")
  6. If it is a command feature, either execute the command or have an exception/return a message that the feature can only run a command
  7. Afterwards, read out the value with feature.get() to check that it was written correctly Cheers, Teresa
NiklasKroeger-AlliedVision commented 2 years ago

The comments made by @Teresa-AlliedVision describe very nicely the intricate problems you will run into with your current implementation. There are multiple different feature types, and with the flexible type system of Python, it is possible that you will sooner or later run into a situation, where you try to set an invalid value type for a feature.

The reason I wanted to implement this at the vimba.Camera level is because I believe such a functionality belongs to a Camera object and I don't want to subclass vimba.Camera to implement this or implement it somewhere else as I'm doing now. It creates some undesired complexity IMHO.

If you want to be able to use the function you provided with the Camera class, this is quite simple with Pythons support for monkey patching. This basically allows you to add new methods to any Python class at runtime. Here is an example similar to what you are trying to achieve. But keep in mind the limitations to your suggested method:

import vimba

# Custom function definition
def my_set_feature_method(cam, feature_name, value):
    print(f'Setting {feature_name} to {value} on {cam}')
    cam.get_feature_by_name(feature_name).set(value)

# Monkey path the custom function into the `vimba.Camera` class as a new method. The `set_feature`
# name is only a suggestion. Method can be named however you want. Can also be identical to custom
# function name. The first parameter of `my_set_feature_method` will be the class instance on which
# the method is called (see example use below)
vimba.Camera.set_feature = my_set_feature_method

with vimba.Vimba.get_instance() as vmb:
    with vmb.get_all_cameras()[0] as cam:
        print(f'Old feature value: {cam.ExposureTime.get()}')
        # Use our monkey patched method to set the value. The `cam` parameter of
        # `my_set_feature_method` is our `cam` instance. This is because the first parameter of a
        # class method is always that class instance.
        cam.set_feature('ExposureTime', 100000)
        # Prove that it worked
        print(f'New feature value: {cam.ExposureTime.get()}')

On my machine with an Alvium 1800 U-500m connected this outputs for example:

Old feature value: 5001.217
Setting ExposureTime to 100000 on Camera(id=DEV_1AB22C00041C)
New feature value: 99998.435

This shows, that our own my_set_feature_method was used by calling the set_feature name we gave it when attaching it to the vimba.Camera class. We did not need to implement any subclass or anything like that. If you want, this kind of patching can even be used to overwrite existing class methods.

A word of caution though: If this is used excessively, it can become quite difficult to tell where certain class methods come from and make it hard to debug applications.

CrocodileDandy commented 2 years ago

@Teresa-AlliedVision 's suggested blueprint has convinced me of necessary work required to implement such a method in a robust way.

Yes @NiklasKroeger-AlliedVision, I am aware that one can monkey patch to bind functions to classes as methods. But for the very reason you mentioned, I try to avoid that kind of practice out of respect for my colleagues and my future self :-D

Would you be interested in a PR that implements @Teresa-AlliedVision suggestion if I ever find time for it or do you consider it out of the scope of the wrapper?