search

Billwilliams1952 / AD9833-Library-Arduino

Library to control the AD9833 waveform generator
114 stars 35 forks source link

Use Library with multiple AD9833 circuits #5

Open virtq opened 6 years ago

virtq commented 6 years ago

Hi, I'm trying to get this library working with multiple AD9833 circuits connected on the same SPI-bus. So I've added the following; AD9833.cpp

void AD9833 :: SetFNCPin ( uint8_t FNCpin ) {
    this->FNCpin = FNCpin;
}

AD9833.h

// Change FNCPin
void SetFNCPin ( uint8_t FNCpin );

However it doesn't really seem to work properly. One of them sometimes get the command that was supposed for the other one, but could perhaps be related to an electrical issue. I just wonder if there's anything more I should do to the library?

Billwilliams1952 commented 6 years ago

I haven’t tried multiple AD9833’s but it should work. I will look at the code and get back with you. It’s a good idea.

Perhaps there is crosstalk between the FNC pin wiring? Ensure the wires are separated enough. Also make sure each AD9833 power line is sufficiently bypassed with 0.1 uF capacitors.

Bill

Sent from my iPhone

On Feb 9, 2018, at 8:36 AM, virtq notifications@github.com wrote:

Hi, I'm trying to get this library working with multiple AD9833 circuits connected on the same SPI-bus. So I've added the following; AD9833.cpp

void AD9833 :: SetFNCPin ( uint8_t FNCpin ) { this->FNCpin = FNCpin; } AD9833.h

// Change FNCPin void SetFNCPin ( uint8_t FNCpin ); However it doesn't really seem to work properly. One of them sometimes get the command that was supposed for the other one, but could somehow be related to an electrical issue. I just wonder if there's anything more I should do to the library?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

Billwilliams1952 commented 6 years ago

I just realized that what you are trying to do is not correct.

To use multiple AD9833s, create an instance for each AD9833, passing the new FNC pin in the constructor. You do not need - or want- a method to change the FNC pin.

Bill

Sent from my iPhone

On Feb 9, 2018, at 8:36 AM, virtq notifications@github.com wrote:

Hi, I'm trying to get this library working with multiple AD9833 circuits connected on the same SPI-bus. So I've added the following; AD9833.cpp

void AD9833 :: SetFNCPin ( uint8_t FNCpin ) { this->FNCpin = FNCpin; } AD9833.h

// Change FNCPin void SetFNCPin ( uint8_t FNCpin ); However it doesn't really seem to work properly. One of them sometimes get the command that was supposed for the other one, but could somehow be related to an electrical issue. I just wonder if there's anything more I should do to the library?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

virtq commented 6 years ago

Thank you for your answer. I actually started out creating an additional instance but since the SPI.begin() is executed within the AD9833.begin() function, I thought it would not be correct to run that function for every instance, since the bus was already started. But perhaps that would be the only problem and I can try to only run that function for the first instance instead.

Billwilliams1952 commented 6 years ago

Let me know what you find out. I’ll modify the source to make the AD9833.begin() a one time call if that is necessary.

Bill

Sent from my iPhone

On Feb 12, 2018, at 3:24 AM, virtq notifications@github.com wrote:

Thank you for your answer. I actually started out creating an additional instance but since the SPI.begin() is executed within the AD9833.begin() function, I thought it would not be correct to run that function for every instance, since the bus was already started. But perhaps that would be the only problem and I can try to only run that function for the first instance instead.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

virtq commented 6 years ago

I was finally able to sort it out. It turned out that it was some kind of electrical issue with the IC, so I had to replace it. Now it works as expected with both code solutions, but I will stick to yours, with creating different instances, because it would probably make the code easier to read.

However, regarding the code library, should perhaps SPI.begin() be moved out from the library completely? Then it would be logically to call Begin() for every instance as you also call the Reset function in there. Alternatively you could have some other function for that in the library, but I don't know what would be the best solution.

Billwilliams1952 commented 6 years ago

I’m glad you worked it out. Electrical bugs can be a pain to troubleshoot.

I will take a look at the library to see where I can improve it.

Good luck with your design!

Bill

Sent from my iPhone

On Feb 13, 2018, at 9:08 AM, virtq notifications@github.com wrote:

I was finally able to sort it out. It turned out that it was some kind of electrical issue with the IC, so I had to replace it. Now it works as expected with both code solutions, but I will stick to yours, with creating different instances, because it would probably make the code easier to read.

However, regarding the code library, should perhaps SPI.begin() be moved out from the library completely? Then it would be logically to call Begin() for every instance as you also call the Reset function in there. Alternatively you could have some other function for that in the library, but I don't know what would be the best solution.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

Billwilliams1952 commented 6 years ago

I looked at spi.Begin(). It merely initializes the SPI pins to their default state. Calling it again should not matter.

As to redefining the FNC pin, I wouldn’t do it. It will work as long as you just call ApplySignal with the same register, but if you use other commands or access both registers, then the internal state of the registers (in the library) will be overwritten.

Creating a new instance for each chip ensures each chip has its own registers that maintain its state.

Bill

Sent from my iPhone

On Feb 13, 2018, at 9:08 AM, virtq notifications@github.com wrote:

I was finally able to sort it out. It turned out that it was some kind of electrical issue with the IC, so I had to replace it. Now it works as expected with both code solutions, but I will stick to yours, with creating different instances, because it would probably make the code easier to read.

However, regarding the code library, should perhaps SPI.begin() be moved out from the library completely? Then it would be logically to call Begin() for every instance as you also call the Reset function in there. Alternatively you could have some other function for that in the library, but I don't know what would be the best solution.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.