Viewer/accept zero exposuretime #821

Fixstars-momoko · 2023-09-19T21:55:56Z

Current arv-viewer accepts >= 1.0 ExposureTime, while GenICam defines its value is >= 0.

This disables the slider when the minimum of ExposureTime is less than 0, so I shifted the value +1.0 so that it makes log10(min) more than or equal to 0.

Reference: https://www.emva.org/wp-content/uploads/GenICam_SFNC_v2_7.pdf

EmmanuelP · 2023-09-20T06:10:37Z

It would be clearer and simpler to clamp the minimum exposure value returned by arv_camera_get_exposure_time_bounds here:

aravis/viewer/arvviewer.c

Line 729 in 73a4ec4

    
           arv_camera_get_exposure_time_bounds (viewer->camera, &viewer->exposure_min, &viewer->exposure_max, NULL);

EmmanuelP · 2023-09-20T06:14:59Z

It would event better to get back to a linear progression if minimum exposure time is <= 0

EmmanuelP · 2023-09-20T06:19:16Z

As a bonus, you may also use the Representation property of ExposureTime.

It is explained page 33 of the Genicam specification: https://www.emva.org/wp-content/uploads/GenICam_Standard_v2_1_1.pdf

Fixstars-momoko · 2023-10-31T22:00:36Z

@EmmanuelP Hi, thanks for the feedback and the info of Representation #821 (comment)

I have updated gtk widget as follows:

Representation	Linear	Logarithmic	PureNumber	Undefined	default /other
edit box	enabled	enabled	enabled	enabled	disabled
slider	enabled	enabled	disabled	enabled	disabled
slider-scale	min-max	0-1	N/A	min-max	N/A

I was not sure in the case of Undefined (-1), so I tentatively enabled both widgets. Please let me know if they are supposed to be disabled.

EmmanuelP

Thanks for the pull request. The comments are mainly coding style issues. Please fix them an I'll merge the changes.

EmmanuelP · 2023-11-01T07:01:40Z

src/arvcamera.c

+ */
+
+ArvGcRepresentation
+arv_camera_get_representation_gain (ArvCamera *camera, GError **error)


Please rename this function to arv_camera_get_gain_representation()
Remove the error placeholder. Errors are for communication problems, not programming errors. get_representation comes directly from the xml data.

updated in b9c1305

EmmanuelP · 2023-11-01T07:02:02Z

src/arvcamera.c

+ */
+
+ArvGcRepresentation
+arv_camera_get_representation_exposure_time (ArvCamera *camera, GError **error)


Rename this function to arv_camera_get_exposure_time_representation()
Remove the error placeholder

updated in b9c1305

EmmanuelP · 2023-11-01T07:03:31Z

src/arvcamera.c

+{
+	ArvCameraPrivate *priv = arv_camera_get_instance_private (camera);
+
+	g_return_val_if_fail (ARV_IS_CAMERA (camera), FALSE);


Type mismatch, return REPRESENTATION_UNDEFINED instead, and update the doc accordingly

Updated in 17a2454

EmmanuelP · 2023-11-01T07:03:33Z

src/arvcamera.c

+{
+	ArvCameraPrivate *priv = arv_camera_get_instance_private (camera);
+
+	g_return_val_if_fail (ARV_IS_CAMERA (camera), FALSE);


Type mismatch, return REPRESENTATION_UNDEFINED instead, and update the doc accordingly

Updated in 17a2454

EmmanuelP · 2023-11-01T07:07:34Z

src/arvdevice.c

+ * Since: 
+ */
+
+ArvGcRepresentation


Remove the error placeholder

updated in b9c1305

EmmanuelP · 2023-11-01T07:12:04Z

viewer/arvviewer.c

+	// gtk_range_set_range (GTK_RANGE (viewer->exposure_hscale), 0.0, 1.0);
+	// gtk_range_set_range (GTK_RANGE (viewer->gain_hscale), viewer->gain_min, viewer->gain_max);


Don't leave commented code

updated in 1fd760c

EmmanuelP · 2023-11-01T07:13:21Z

viewer/arvviewer.c

+	gint gain_representation;
+	gint exposure_time_representation;


Wrong type, use ArvGcRepresentation

updated in c5ccba1

EmmanuelP · 2023-11-01T07:13:58Z

viewer/arvviewer.c

-	gtk_widget_set_sensitive (viewer->gain_hscale, is_gain_available);
-	gtk_widget_set_sensitive (viewer->gain_spin_button, is_gain_available);
+	if (is_gain_available){
+		viewer->gain_representation = (gint)(arv_camera_get_representation_gain(viewer->camera, NULL));


Remove the cast to gint

updated in c5ccba1

EmmanuelP · 2023-11-01T07:14:41Z

viewer/arvviewer.c

-	gtk_widget_set_sensitive (viewer->exposure_hscale, is_exposure_available);
-	gtk_widget_set_sensitive (viewer->exposure_spin_button, is_exposure_available);
+	if (is_exposure_available){
+		viewer->exposure_time_representation = (gint)(arv_camera_get_representation_exposure_time(viewer->camera, NULL));


Remove the cast to gint

updated in c5ccba1

EmmanuelP · 2023-11-01T07:14:44Z

viewer/arvviewer.c

@@ -749,17 +767,66 @@ set_camera_widgets(ArvViewer *viewer)
 	string = g_strdup_printf ("%g", arv_camera_get_frame_rate (viewer->camera, NULL));
 	gtk_entry_set_text (GTK_ENTRY (viewer->frame_rate_entry), string);

+	// enables gtk widget based on feature representation


Please remove the comment, the code is self explanatory

updated in 1fd760c

EmmanuelP · 2023-11-02T15:02:09Z

Thanks a lot. I have merged your changes with some minor fixes.

I may revert the widget selection based on representation though, at least for exposure time, as it looks like most of the time it is defined as linear, while logarithmic makes more sense.

Fixstars-momoko added 2 commits September 20, 2023 06:40

allows ExposureTime (GenICam feature) to have 0 as minimum value

398fb88

Merge branch 'main' into viewer/accept-zero-exposuretime

cc68934

EmmanuelP marked this pull request as draft September 20, 2023 06:10

Fixstars-momoko added 4 commits November 1, 2023 03:42

added API obtaining representation of integer/float node

4b99ed6

added API to check representation for gain and exposuretime

76bde7b

enabled gtk widget based on feature representation

8ffe240

confirmed with representation of gain = 3; exposure = -1

eec3ba5

Fixstars-momoko marked this pull request as ready for review October 31, 2023 22:22

EmmanuelP requested changes Nov 1, 2023

View reviewed changes

Fixstars-momoko and others added 7 commits November 2, 2023 02:15

rename functions and remove unnecessary error placeholders

b9c1305

fixed return type

17a2454

fixed return type (additionally)

f2ce842

remove unnecessary lines

1fd760c

fixed the type

c5ccba1

fixed the node type

aca191d

Merge branch 'AravisProject:main' into viewer/accept-zero-exposuretime

dc12ac1

EmmanuelP closed this Nov 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Viewer/accept zero exposuretime #821

Viewer/accept zero exposuretime #821

Fixstars-momoko commented Sep 19, 2023

EmmanuelP commented Sep 20, 2023

EmmanuelP commented Sep 20, 2023 •

edited

Loading

EmmanuelP commented Sep 20, 2023

Fixstars-momoko commented Oct 31, 2023

EmmanuelP left a comment

EmmanuelP Nov 1, 2023

Fixstars-momoko Nov 1, 2023

EmmanuelP Nov 1, 2023

Fixstars-momoko Nov 1, 2023

EmmanuelP Nov 1, 2023

Fixstars-momoko Nov 1, 2023

EmmanuelP Nov 1, 2023

Fixstars-momoko Nov 1, 2023

EmmanuelP Nov 1, 2023

Fixstars-momoko Nov 1, 2023

EmmanuelP Nov 1, 2023

Fixstars-momoko Nov 1, 2023

EmmanuelP Nov 1, 2023

Fixstars-momoko Nov 1, 2023

EmmanuelP Nov 1, 2023

Fixstars-momoko Nov 1, 2023

EmmanuelP Nov 1, 2023

Fixstars-momoko Nov 1, 2023

EmmanuelP Nov 1, 2023

Fixstars-momoko Nov 1, 2023

EmmanuelP commented Nov 2, 2023

		// gtk_range_set_range (GTK_RANGE (viewer->exposure_hscale), 0.0, 1.0);
		// gtk_range_set_range (GTK_RANGE (viewer->gain_hscale), viewer->gain_min, viewer->gain_max);

Viewer/accept zero exposuretime #821

Viewer/accept zero exposuretime #821

Conversation

Fixstars-momoko commented Sep 19, 2023

EmmanuelP commented Sep 20, 2023

EmmanuelP commented Sep 20, 2023 • edited Loading

EmmanuelP commented Sep 20, 2023

Fixstars-momoko commented Oct 31, 2023

EmmanuelP left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EmmanuelP commented Nov 2, 2023

EmmanuelP commented Sep 20, 2023 •

edited

Loading