adafruit / Adafruit_Si5351_Library

Driver for Adafruit's Si5351 Clockgen Breakout
43 stars 30 forks source link

Multisynth function updates, 400kbps i2c, burst send mode #7

Closed sx107 closed 3 years ago

sx107 commented 3 years ago
ladyada commented 3 years ago

hihi please remove the setClock() - you can call that after begin() in your main app.

sx107 commented 3 years ago

hihi please remove the setClock() - you can call that after begin() in your main app.

Won't it completely ruin the incapsulation? Seems weird to be referencing the global class Wire from "outside", since it is essentialy initialized inside the library (wire.begin). Perhaps passing a wire instance to the constructor (which will also add support for uCs with multiple i2cs in the future), or adding a setSpeed(low/high) method, or leaving it as it is l, but with capability to change the speed in the constructor would make sense?

If not, then the maximum speed supported by Si5351 should be at the very least mentioned in the example by adding a single Wire.setClock line right after si5351 initialization with a comment that "this is the maximum speed, however, lower speeds are also supported and preferable if stability is a major concern."

ladyada commented 3 years ago

Wire is a singleton, you can set the clock speed in your main program and it will change the speed for every sensor. technically we should also allow passing in a twoWire object by reference - we do for most other drivers (this one is old so that wasnt a Thing Folks Did)

sx107 commented 3 years ago

technically we should also allow passing in a twoWire object by reference - we do for most other drivers (this one is old so that wasnt a Thing Folks Did)

Yeah, sorry, that's what I meant. Well then, maybe for the moment I just add another line (Wire.setClock(400000)) in the example to state that the maximum speed is 400kbps?

ladyada commented 3 years ago

you can put that in the example yeah. please comment it out and tell folks they can uncomment to speed up!

ladyada commented 3 years ago

ok lastly, what hardware did you use to test?

sx107 commented 3 years ago

Arduino Nano, Si5351 module clone from aliexpress. Since the modifications affect only the i2c communication itself, I doubt that replacing the arduino nano with any other will break the code. All modifications were made according to the Si5351 datasheet.

ladyada commented 3 years ago

ok we'll see if anyone has issues in which case we'll revert!