[gphoto-patches] [ gphoto-Patches-2819010 ] Bunch of ptp/canon_eos related improvements

[SourceForge.net]

Patches item #2819010, was opened at 2009-07-09 13:00
Message generated for change (Settings changed) made by marcusmeissner
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=308874&aid=2819010&group_id=8874

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: None
Group: None
>Status: Closed
>Resolution: Fixed
Priority: 5
Private: No
Submitted By: Axel Waggershauser (awagger)
Assigned to: Marcus Meissner (marcusmeissner)
Summary: Bunch of ptp/canon_eos related improvements

Initial Comment:
To Marcus and whoever is interested in ptp stuff... The attached set of patches contains:

+ random-warning-fixes.patch
    (just what the name says)

+ remote-release-doc.patch
    (simple two-liner explaining the EOS_RemoteReleaseOn/Off commands)

+ 5D-mark-ii-iso-and-fstop.patch
    (some additional iso and fstop values used by the 5D Mark II)

+ canon-eos-aeb-and-drive-mode.patch
    (support for the AEB and DriveMode properties of EOS cameras)

+ canon-eos-string-and-read-only-int-properties.patch
    (string properties like Owner/LensName/etc. and two read-only int properties AvailableShots/ShutterCounter)

+ canon-eos-custom-func-debug-output.patch
    (cosmetically improve / semi interpret the custom-func props of the 400D)

+ improve-debugging-output.patch
   (arguably improving debugging output especially for the "special" EOS-ImageFormat-type of enumerations)

These patches might be applicable in any order and in any subset selection.

----------------------------------------------------------------------

>Comment By: Marcus Meissner (marcusmeissner)
Date: 2009-10-31 19:31

Message:
lets close for now. :)

thanks for the work, if something new happens open a new bug :)

----------------------------------------------------------------------

Comment By: Marcus Meissner (marcusmeissner)
Date: 2009-07-16 16:28

Message:
It is implemented via setting because you cannot find out from the camera
in which capture mode it is.

The Canon and Nikon capturetargets are implemented the same way btw,
since
they also can't store this on the camera.



I applied your patch, I just replaced the "uint16_t" type of the call with
"int".

----------------------------------------------------------------------

Comment By: Axel Waggershauser (awagger)
Date: 2009-07-16 15:13

Message:
Another patch (capturetarget-online-change-support.patch) this time, even
including the ChangeLog update :-). It allows to do something like this:

    ./gphoto2 --set-config capturetarget="Memory card" --capture-image
--set-config capturetarget="Internal RAM" --capture-image

It also has some cleanup aspects to it. The removal of a couple of
ptp_canon_eos_pchddcapacity calls might be considered to be regression
risk. Although I doubt that it is necessary to call that function so often,
just because the EOS Utility does. In fact, the current version of the EOS
Utility calls this function in yet another pattern than the one that is
currently checked in. I'll add a trace log of a complete 'start-up'
sequence with an EOS 5D Mark II, for the interested reader.

By the way: why has this capturetarget property been implemented to
differently? I mean the fact, that it is saved in the settings file?

The patch is tested on EOS 5D Mark II and EOS 400D.

----------------------------------------------------------------------

Comment By: Axel Waggershauser (awagger)
Date: 2009-07-12 21:08

Message:
Thanks for the explanation, I'll try to remember... :-)

----------------------------------------------------------------------

Comment By: Marcus Meissner (marcusmeissner)
Date: 2009-07-12 20:48

Message:
I applied the 2 new ones.

The battery one had to use _() instead of N_().

N_() just marks up the string for translation, but does not translate it
instantly.
_() does. So N_() is usually used in tables (where the string is not used
immediately),
_() when it is required translated immediately.

----------------------------------------------------------------------

Comment By: Axel Waggershauser (awagger)
Date: 2009-07-12 20:39

Message:
Two new patches:

+ ptp-debugging-output-cosmetic.patch
reduce the debugging messages in camera_set_config. it feels a bit more
consistent to me now...

+ canon-eos-battery-property.patch
should be obvious (I did not model it as an enumeration since it is
inherently 'read-only'. better idea?)

----------------------------------------------------------------------

Comment By: Marcus Meissner (marcusmeissner)
Date: 2009-07-10 17:12

Message:
i applied both new ones, thanks!

no better idea right now.

i am cuyrrently working on the generic functions

----------------------------------------------------------------------

Comment By: Axel Waggershauser (awagger)
Date: 2009-07-10 16:10

Message:
Two new patches:

+ canon-eos-image-format-property.patch
This code works with both 400D and 5DMii, i.e. it is probably general
enough to work with every EOS device, as I am not aware of any camera
having more ImageFormat options than the 5DMii. It is, however, not very
elegant. I opted for the 'one-property' solution as it seemed more
consistent and easier to implement. I sort of 'hacked' the custom image
format property values into a uint16_t to be able to use the existing
infrastructure. This resulted in some 'special' handling - you might have a
better idea...

+ fix-drive-mode-enumeration.patch
pretty obvious two-liner

Ad generic commands:
I did not really like the varargs version either. I'd rather prefer
classic c++-style function overloading to be able to omit the n_params
parameter. Which is of course only one possible source for trouble. The
other (just occured to me) being the fact, that the caller has to know how
many parameters the command expects. Now I am convinced, this is bad.

You probably meant something like this?!?:
#define ptp_canon_eos_requestdevicepropvalue(param, prop) \
        ptp_generic_no_data (param,
PTP_OC_CANON_EOS_RequestDevicePropValue, 1, prop)

I'd appreciate that.


P.S. to hfiguiere: you are absolutely right about the spelling of "bunch",
thanks :-)

----------------------------------------------------------------------

Comment By: Marcus Meissner (marcusmeissner)
Date: 2009-07-10 06:47

Message:
as for generic commands.

i am thinking about it. I am not sure I like the calling varargs directly
solution, I would then use something like

#define ptp_canon_eos_requestdevicepropvalue(p,prop) ptp_generic_no_data
(p, 1, prop)

and then call this. This ensures argument caller safety at least.

----------------------------------------------------------------------

Comment By: Marcus Meissner (marcusmeissner)
Date: 2009-07-10 06:40

Message:
you can add them there, and we should remove the ones already applied.

----------------------------------------------------------------------

Comment By: Axel Waggershauser (awagger)
Date: 2009-07-09 22:38

Message:
I started implementing your recommendations and was about half way
through... ;-)

Regarding your suggestion to implement a
ptp_canon_eos_requestdevicepropvalue function. That is what I did first,
actually. Then I had to add another two while experimenting with the
RemoteReleaseOn/Off commands that looked _very_ similar (obviously) and I
am a fan of neither code bloat nor redundancy. Also, the fact, that one
sometimes has to check if a given command is available at all (as in this
case) and therefore has to know the name of the respective op-code anyway,
means one has to know two names for the same thing.

Two make a long story short: I am not convinced (yet ;-)) that the
approach of having a separate function for each and every command is the
best solution.

Anyway, there is more to come, like support for the ImageFormat property.
Should I just add that here or open a new patch-tracking-ticket?

----------------------------------------------------------------------

Comment By: Marcus Meissner (marcusmeissner)
Date: 2009-07-09 20:50

Message:
I ported the last two patches to the style I would recommend and applied to
both 2.4 branch and TRUNK.

Thanks:)

----------------------------------------------------------------------

Comment By: Marcus Meissner (marcusmeissner)
Date: 2009-07-09 16:49

Message:
i applied:
+ random-warning-fixes.patch
 (just what the name says)
 
 + remote-release-doc.patch
 (simple two-liner explaining the EOS_RemoteReleaseOn/Off commands)
 
 + 5D-mark-ii-iso-and-fstop.patch
 (some additional iso and fstop values used by the 5D Mark II)
 
+ canon-eos-custom-func-debug-output.patch
 (cosmetically improve / semi interpret the custom-func props of the
 400D)
 
to TRUNK now. Will also do for 2.4 branch later on.

canon-eos-string-and-read-only-int-properties.patch:
- please define a ptp_canon_eos_requestdevicepropvalue() fuinction in
ptp.c/ptp.h and use it.
  (generic functions are too difficult to handle)

- get_INT() should have error return in the default case, and create the
widget after the switch()

canon-eos-aeb-and-drive-mode.patch:
- we cannot pas a UINT32 dpd into the 16bit table function, it will not
work on big endian machines. We basically either need to duplicate the 16
-> 32bit table function ... or check if we cannot express the values as
16bit values anyway in ptp-pack.c



----------------------------------------------------------------------

Comment By: Axel Waggershauser (awagger)
Date: 2009-07-09 13:07

Message:
I forgot to mention that they are based on the current version in the 2_4
svn branch. The only "tested" order of application is the one given in the
details above.

----------------------------------------------------------------------

You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=308874&aid=2819010&group_id=8874

------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
gphoto-patches mailing list
gphoto-patches@...
https://lists.sourceforge.net/lists/listinfo/gphoto-patches