Here’s my thoughts:
sendACK() should use 2 arguments and not just one, in fact it acknowledge a command, so you should put a ‘command’ argument as a first argument, for example last ACK for getRawPicture should be AA 0E 0A 00 00 00, instead of AA 0E 00 00 00 00 as you’ve put.
Good point, per your argument about simplifying the code in SYNC. I think I had planned on doing this, and then forgot about it.
again on SYNC, for getJPEGPicture, the last ACK is a special ACK that acknowledge the last package, this ack should be AA 0E 00 00 F0 F0, whatever the packageCount may be
This is what getJPEGPicture is doing, line 260. Am I missing something? In the manual, F0F0h seems to be just an example packageId number, correct?
I don’t know well yet the wire language, but I think you can use lowByte() and highByte in functions sendAck(), setPackageSize(), snapshot() instead of p & 0xFF, p >> 8
I’ll look into this. I’m not too familiar with all the Wiring functions either. I imagine the savings would be minimal though …
I don’t know if arduino supports Serial.begin() several times to change the baudrate during communication, but if yes, I think you should add a Serial.begin(baudRate) in setBaudRate()
It does … but I was attempting to leave this up to the programmer using the library. So that he/she calls Serial.begin() after a return true from the baud rate change. Ultimately, I’d really like to reduce reliance on Serial altogether, so that developers have the option of using the hardware or a software serial library like NewSoftSerial.
Do you think more control should be built into the library?
In the documentation (FAQ), it’s said that “After synchronization, the camera needs a little time for AEC and AGC to be stable. Users should wait for 1-2 sec before capturing the first picture”
=> I think you should add a delay(2000) at the end of the sync() function
Same as above … I was leaving this up to the developer so that I don’t force a 2 second delay (maybe they want to use that 2 seconds to do processing?) But I suppose it’s not a bad idea … my testing with the camera was that waiting 2 seconds here made no difference anyway, but probably good to add.
Maybe there’s a rare bug that may occurs in waitForResponse(), indeed millis() goes back to 0 every 52 days, so millis() - time may give bad result at the end of the 52 days
My understanding is that delay() internally uses millis() No? Is this bug avoidable? I’ll do some more research.
Maybe it would be an enhancement of the lib, in fact in getJPEGPicture, the package given to the callback contains 6 bytes which are not jpeg data (the user needs to drop 4 first bytes and 2 last bytes of the package to get the data)
Agreed. I was planning on doing this but wanted to get the code out to you for testing.
last thing, i’m not so sure, but in the documentation, it says to do the sync at 14 400bps, however i’ve done some tests at 9600, it works…
Hmm … it seems to sync no matter what baud rate you use. I’ve tried 9600, 14400, 38400 …
Good points, all of them! I’ll make the changes and post a new version, or if you have made them already, feel free to send the modified files.