Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rudimentary GenTL interface #828

Closed
wants to merge 19 commits into from
Closed

Conversation

eudoxos
Copy link
Contributor

@eudoxos eudoxos commented Oct 14, 2023

Hi there, I have an initial implementation of GenTL.

It is far from complete, but perhaps a good start. I put it here more for discussion, not for merging.

Harvesters can already use it to enumerate devices (test-01-load.py script) (see here for trace output: harversters-update.log; had to fix Harversters logic at one place, though genicam/harvesters#435 ).

Code style (in gentl/*.c) terrible, I am counting on auto-format to polish that later.

Some minor enhancements were needed in Aravis itself (more will be needed later).

@EmmanuelP EmmanuelP marked this pull request as draft October 15, 2023 06:44
@xiaoqiangwang
Copy link

Just want to say I have been working on GenTL support as well and tested it to work with Pylon GenTL producer ProducerU3V.cti. https://github.com/xiaoqiangwang/aravis/tree/gentl

I meant to polish the GenTL handle management and API wrapper a bit more before announcing it. I hope it can be a useful source still.

@EmmanuelP
Copy link
Contributor

Just want to say I have been working on GenTL support as well and tested it to work with Pylon GenTL producer ProducerU3V.cti. https://github.com/xiaoqiangwang/aravis/tree/gentl

I meant to polish the GenTL handle management and API wrapper a bit more before announcing it. I hope it can be a useful source still.

Hi @xiaoqiangwang ,

I was worrying about how to test @eudoxos work, it looks like you have brought the solution. Thanks a lot for your work. Please open a pull request. I don't have much time now for a thorough review, but your implementation seems pretty neat. The only things I have seen so far that could be problematic are the stream_start/stop functions. I hope it can be avoided.

@xiaoqiangwang
Copy link

I will finish the polishing I have in mind and ask for a review.

The only things I have seen so far that could be problematic are the stream_start/stop functions. I hope it can be avoided.

Me too worried about this breaking change. However the GenTL producer requires these two actions. One way to hide them from the user is to call them when Stream object is constructed/finalized.

@EmmanuelP
Copy link
Contributor

I was worrying about how to test @eudoxos work, it looks like you have brought the solution.

To clarify what I meant, my worry was about the CI pipeline. I was not sure how we could test @eudoxos 's GenTL provider. But I was forgetting the harvesters project, which can perfectly be integrated in the CI pipeline.

@EmmanuelP
Copy link
Contributor

Hi @eudoxos

Do you have pending update to this pull request ? I'm tempted to merge it as it is. It does not pull any new dependencies and does not interfere with the rest of aravis.

@eudoxos
Copy link
Contributor Author

eudoxos commented Nov 21, 2023

Hi @EmmanuelP, I commited some WIP that I have locally. I would be glad for it to be merged, as I seem to have reached limits of my technical knowledge of Aravis and GenTL — GenTL events don't seem to have a direct counterpart in Aravis, so there are some stubs prepared for that. Harversters can open the camera all way to creating the stream, where stream and events would be needed already.

@xiaoqiangwang
Copy link

In the TLGetNumInterfaces and TLGetInterfaceID methods, it is best to exclude the GenTL consumer interface., because it would create infinite loops if aravis GenTL consumer interface loads aravis GenTL producer. Also whoever uses aravis as a GenTL producer is itself already a GenTL consumer.

@EmmanuelP EmmanuelP marked this pull request as ready for review November 22, 2023 07:58
@EmmanuelP
Copy link
Contributor

Merged in main. I did not try to include you last commit 87e2c49. Please let me know if you find a way to make the build pass for every platform. I did only activate gentl producer build on linux ci pipeline.

Thanks a lot.

Event is not yet supported in aravis, even though there is an implementation in @xiaoqiangwang gentl interface. I still have to think about the API.

The most important thing is DataStream support. For what I have understood while reviewing @xiaoqiangwang work, it should be doable. If there is a need for an Aravis API break, it is the right time, I'm starting a 0.9 unstable release series.

@EmmanuelP EmmanuelP closed this Nov 22, 2023
@EmmanuelP
Copy link
Contributor

EmmanuelP commented Nov 22, 2023

Oh, by the way, when I try to launch one of the harvesters tests, I get the following error:

Traceback (most recent call last):
  File "/home/pacaud/Sources/aravis/build/../tests/python/arv-gentl-producer-load-test.py", line 5, in <module>
    H.add_file(os.environ.get('ARV_PRODUCER_PATH'))
    ^^^^^^^^^^
AttributeError: 'Harvester' object has no attribute 'add_file'

This is with harvesters installed today using pip3.

@EmmanuelP
Copy link
Contributor

In the TLGetNumInterfaces and TLGetInterfaceID methods, it is best to exclude the GenTL consumer interface., because it would create infinite loops if aravis GenTL consumer interface loads aravis GenTL producer. Also whoever uses aravis as a GenTL producer is itself already a GenTL consumer.

I have disabled the GenTL interface in the producer init code.

Thanks for the tip.

@eudoxos
Copy link
Contributor Author

eudoxos commented Nov 22, 2023

AttributeError: 'Harvester' object has no attribute 'add_file'

I was just following the tutorial and it mentions this method. I can run ARV_DEBUG=all:5 ARV_PRODUCER_PATH=../../build/src/gentl/aravis-0.8.31.cti python3 arv-gentl-producer-nodes-test.py without this issue. Harvesters updated from pip3, version 1.4.2). Strange you see this error.

@EmmanuelP
Copy link
Contributor

That is because pip3 installed a very ancient version of Harvesters. It is the only available version on fedora 39, probably due to a too recent python that prevents the installation of genicam2.

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.

5 participants