Fix exposure mode handling bugs.

[sh0]

There are some weird things going on with pslr_exposure_mode_t and pslr_gui_exposure_mode_t.

My understanding is that there was a commit 582e03f7926314ee6fbec4f1b053e222d96cb6e0 (Wed May 29 18:04:27 2013 +0000) which added separate GUI exposure mode enum pslr_gui_exposure_mode_t, but the conversion between enums was forgotten in user_mode_combo_changed_cb and/or pslr_set_exposure_mode. Then recently this issue was discovered and commit f9e23edcf88bfc5ca356bbfce91ca085aef56e54 (Sun Mar 22 14:03:39 2020 +0100) tried to fix it by adding enum conversion to pslr_set_exposure_mode. However the conversion is done in the wrong direction.

To try to summarize, currently:

• pktriggercord.c calls pslr_set_exposure_mode with pslr_gui_exposure_mode_t argument.
• pktriggercord-cli.c calls pslr_set_exposure_mode with pslr_exposure_mode_t argument.
• pslr.c function pslr_set_exposure_mode incorrectly converts pslr_exposure_mode_t to pslr_gui_exposure_mode_t. In CLI case this is incorrect as the enum should be left to pslr_exposure_mode_t and in GUI case the mode is already in pslr_gui_exposure_mode_t type so it does the conversion twice.

I think the whole mode switching code was buggy before need_exposure_mode_conversion flag was added for K-30 camera. This can also be seen from the issue #52 that something is wrong. So I think that the K-30 handling was probably not needed in the first place.

My experimental fix is as follows:

• pslr.c keeps track of camera mode only using pslr_exposure_mode_t type. This is preferred over pslr_gui_exposure_mode_t as it has more entries and is more accurate.
• CLI pktriggercord-cli.c code mostly operates with pslr_exposure_mode_t type except few PSLR_GUI_EXPOSURE_MODE_B compares which were changed.
• GUI pktriggercord.c now converts between pslr_gui_exposure_mode_t and pslr_exposure_mode_t as needed.
• pslr.c has new functions pslr_convert_exposure_mode_to_gui and pslr_convert_exposure_mode_from_gui instead of single exposure_mode_conversion function.
• need_exposure_mode_conversion structure entry and pslr_get_model_need_exposure_conversion function are removed.

I don't have Pentax cameras so someone else would need to do tesing.

[asalamon74]

Thanks, I'll need more time to check this. This part of the code is a mess, definitely, it would be great to clean this up.

[asalamon74]

I checked a few things and the history of this fields:

• PkRemote (the project I forked to create pkTriggerCord) has only one enum for exposure modes. The name is pslr_exposure_mode_t but the values are the same as the pslr_gui_exposure_mode_t in pkTriggerCord https://sourceforge.net/p/pkremote/code/HEAD/tree/pslr.h
• The first version of pkTriggerCord already contains the two enums and the conversion https://github.com/asalamon74/pktriggercord/blob/35cec143a5778c755afcb70e0fe483bde4706620/pslr.h (it means that this mess was introduced by me)
• The 7d561e9a357d563a7b19fc7584ec678a6028d9fc added more values to pslr_exposure_mode_t.
• The 582e03f7926314ee6fbec4f1b053e222d96cb6e0 commit only turned this off for K30.

The values of pslr_exposure_mode_t are rather similar to the PictureMode exif field ( https://exiftool.org/TagNames/Pentax.html ) the values of pslr_gui_exposure_mode_t more resembles the values printed on the camera.

So why was this conversion added by me when I added the support for K-x? I don't really remember, but probably I realised that when I set the camera to M mode I read the value 8 instead of the expected 6. Setting exposure values was not possible for K-x, so I couldn't check that part.

Why do we need this at all? I tested the exposure mode setting using K-70. It only works if I set the camera to user mode. Let's say I want to change to Sv mode. The value is 15 in pslr_exposure_mode_t and 2 in pslr_gui_exposure_mode_t. I have to send the value 2 to the camera (I turned off all the conversions for the test). The camera will change to Sv mode, and reading back will get me the value of 15. So it seems to me, the camera has different values for reading or writing this field (but why?).

So it means that pslr_set_exposure_mode should either get the gui value or convert the non-gui value before sending the value to the camera. It is clearly wrong that in case of the GUI we have a double conversion. It is also true that it's better to store the non-gui version values in pslr.c since it has more values.

[sh0]

Thanks for taking a look. I'm also not quite sure what the best option is here.

I don't known how to handle all the user mode value conversions, but it feels to me that having only pslr_exposure_mode_t in pslr.c would at least simplify the API. Last time I looked, it seemed like libgphoto2 was also using the exposure mode settings incorrectly because it was mixing GUI and non-GUI enum values. So some clarity from the user-facing aspects would be nice.

For K-30 camera the issue #52 was somewhat promising that my patch allowed the exposure mode to be correctly recognized. I don't really remember all the details though.