Smoothieware / Smoothie2-old

(Deprecated attempt at) A Smoothie firmware port to the Smoothieboard v2 boards.
GNU General Public License v3.0
34 stars 22 forks source link

ADC Busrt Mode #27

Closed marcosfg closed 8 years ago

marcosfg commented 8 years ago

The current push request includes changes to configure the ADC in Burst mode. This implementation followed the "libs/ADC/adc" implementation from smoothie v1.

Currently known limitations and TODOS:

arthurwolf commented 8 years ago

@adamgreen Do you have an opinion on what the problem could be here ? @marcosfg has made huge progress towards porting the ADC stuff, has it working, is now working on implementing "burst mode" ( where the chip reads the ADC all the time in the background, and we just ask it for the latest value, so we don't have to wait for it to read the actual peripheral ). However, he seems to have a problem with the interrupt there.

@marcosfg Could you give us as much information as possible as to what you tried, and exactly how it breaks ?

marcosfg commented 8 years ago

To enable interrupts you should change the line 62 of the libs/ADC.cpp to:

this->adc->interrupt_state(pin_name, 1);

This enables the interrupt, and once an A/D ISR occurs it calls the BurstADC::adcisr(). After entering this ISR it never clears the interrupt flag and loops here forever.

According to the chip manual section 47.7.2:

"The result register of the A/D channel which is generating an interrupt must be read in order to clear the corresponding DONE flag"

The current code reads the A/D STATUS register to scan for over-run or done events. Then it reads the corresponding channel result register (DR[channel])

However the interrupt flag it is not cleared. I have also tried to read every other A/D register and the DONE, OVER-RUN or ADINT bits are not cleared.

arthurwolf commented 8 years ago

@marcosfg : @adamgreen is going to be away for a while, I was hoping for him to help here. He'll help as soon as he comes back.

@wolfmanjm this sounds like something that'd be easy for you to figure out. Would you mind taking a look ?

arthurwolf commented 8 years ago

Also tagging @triffid and @logxen while at it.

triffid commented 8 years ago

Perhaps it works differently in burst mode, and you have to clear the flag manually? Haven't checked datasheet for LPC43xx but some chips require you to write a word with the flag bit set to 1 to clear the flag, which should avoid race conditions on other bits in the word changing during a read-modify-write.

If the ADC runs continuously, what meaning does the DONE flag have?

You could use DMA to feed it all into a buffer for use with oversampling and median cut and suchforth, then flip buffer and process contents when the DMA task is complete

On 14 August 2016 at 23:15, Arthur Wolf notifications@github.com wrote:

Also tagging @triffid https://github.com/triffid and @logxen https://github.com/logxen while at it.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Smoothieware/Smoothie2/pull/27#issuecomment-239678981, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKBGhVPHRFsO866VpQbPzD9Y67NHOe9ks5qfzEagaJpZM4JiIb2 .

arthurwolf commented 8 years ago

Oh that actually makes me wonder : what's that about interrupts ? The whole point about bust mode is the ADC is just read continuously in the background, and we can read the value whenever we want without having to wait for the peripheral to do it's job. Where does an interrupt fit in that picture ?

marcosfg commented 8 years ago

According to the datasheet an interrupt is triggered when any of the channels conversions is done. Interrupt flag is located at STATUS register which is read only. I did howerver tried to clear it manually.

Previous implementation from SmoothieV1 was using interrupts to store the conversion results of the last 8 values from each channel. These 8 values are then used to retrieve a median value.

Interrupts allow that every time a new conversion ends, its buffer is updated with the last conversion value.

Current code launches a slow_ticker to get the temperature value 20 times a second (default value defined at config). We could try to increase the readings per second, and use this call to apply the filters.

An interrupt based approach (be its source ADC or DMA) would allow a faster response.

arthurwolf commented 8 years ago

I understand that the interrupt is triggered when the conversion is done. However, the way i understand burst mode, the whole point is that you don't care about that : the conversion is done in the background automatically by the microcontroller, you don't have to have an interrupt, the microcontroller does all that work for you, that's not code you are supposed to do. Am I missing something ?

marcosfg commented 8 years ago

correct, however I don't think it is a good idea to use the last reading from the ADC. The last reading may contain bad readings from interferences.

There should be a routine to periodically read the ADC value and apply some filter to remove noise. This was being done using the adc interrupt. O believe we could obtain the same results using a timer.

arthurwolf commented 8 years ago

Smoothie already has code to filter out bad readings from interference. The v1 code reads the last value from burst mode, and does it's thing with it ( including averaging and weeding out bad values ). You don't need to do anything more than v1 does here, it's all there already, you just need burst mode to work, the normal temperaturecontrol timer to work, and it'll all work as well as v1 works. No ?

marcosfg commented 8 years ago

Correct me if I'm wrong.. In v1 the adc interrupt calls custom isr "sample_isr" from Adc.cpp. This routine then shifts the last 7 values in the sample_buffer and stores the new reading.

The filter in v1 then uses this buffer to calculate the median value. By removing the interrupt, the buffer manipulation task should be done every time a new reading is done.

I will implement this method for the ADC Busrt mode and proceed with other ports. In the meantime it can be possible that by working on other peripherals I'll be able to find out what is wrong.

arthurwolf commented 8 years ago

@wolfmanjm I'm not actually clear what the v1 code actually does there. The only interrupt that is used is SlowTicker right ?

marcosfg commented 8 years ago

No. Currently v1 uses 2 interrupts for the adc readings:

1-The SlowTicker (default 20 times a second for each sensor) uses the values stored in the samples buffer and calculates the meadian value.

2-The ADC interrupt, is called every time a conversion is done. Then the isr routine call a custom isr in Adc.cpp to move the values in the samples buffer and store the new value.

arthurwolf commented 8 years ago

Ah ok that's what I didn't understand. To me the whole point of burst more was that you didn't need the second interrupt ...

marcosfg commented 8 years ago

I have included the following changes:

-Adc::read now fills and moves the sample buffer. Calculates the adc value using oversampling. -Default readings per second changed to 160.

This changes allows the ADC to work in burst mode. My thoughts on its current limitation:

-By disabling interrupts, the adc readings becomes less responsive (I have registered about 1second to update new values). For these reason I have increased it default value to 160 (8x the previous default value) -This reflects also in an increase in processing needs.

TODO: -Find out what is wrong with interrupt. This allows the buffer to be updated in the background, the temperature calculation could then be reduced again to the default 20 readings per second.

arthurwolf commented 8 years ago

I'm merging this as it's progress, and the only thing that needs to be fixed is the interrupt issue.

arthurwolf commented 8 years ago

Thank you very much !