From: Wolfram S. <wo...@th...> - 2013-08-20 17:44:56
|
> The issue is that clearing the 0x20 bit seems to mark it as "downloaded", > so the code change is probably bad. Yes, see later. > I think it actually misses a query of the old attribute here and a adjustment > after that. Not really, since the original attributes are converted to the gphoto filesystem which the function queries. I digged some more, here my findings: * CANON_ATTR_DOWNLOADED is a misguiding name, since a _set_ bit means _not downloaded_. The macro should have been named CANON_ATTR_NOT_DOWNLOADED. * So, the original code was okay from a logic point of view, it *set* the bit when the file was *not downloaded*. * Commit 7691 then confuses the logic AFAICS. It *clears* the bit, when the file was *not downloaded* (which is especially bogus since attr was 0 always). * However, in this place we should not ask the gp filesystem about the status AFAICS. This is the place where we have to *tell* the filesystem that the file was successfully downloaded. The cameras don't do this automatically (at least the 350D here) and I can't find it being done somewhere else. So, this is where I wonder about the original code, too. What currently happens: * Due to the bogus check, the CANON_ATTR_DOWNLOADED will always be cleared (thus, marked as downloaded). * We don't notify the gp filesystem, but it probably gets updated after directories are read the next time. * It makes sense to clear the bit, yet we have a protocol problem with the 350D and 5D and I need to postpone this to somewhen later. Still, I'd like to suggest two patches. I might be on IRC later, too. Thanks, Wolfram |