8000 Fix crash on audio end from IRQ, refactor A2DP (#2189) · 12a318/arduino-pico@bf38549 · GitHub
[go: up one dir, main page]
Skip to content

Commit bf38549

Browse files
earlephilhowerearlephilhower
authored
Fix crash on audio end from IRQ, refactor A2DP (earlephilhower#2189)
Fixes earlephilhower#2188 We get a call to stop the audio channel from a timer/IRQ context, so can't safely remove the IRQ handler for the AudioBufferManager. The SDK will panic. Because the IRQ handler will be a noop if it's not uninstalled, we will instead just leave our shared handler in place and let it do nothing. Use a common BluetoothLock RAII in BluetoothAudio to clen up the code and automatically lock BT for the AVRCP button methods.
1 parent 919a754 commit bf38549

File tree

9 files changed

+80
-35
lines changed

9 files changed

+80
-35
lines changed

‎libraries/AudioBufferManager/src/AudioBufferManager.cpp

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,9 @@
2424
#include <hardware/irq.h>
2525
#include "AudioBufferManager.h"
2626

27-
static int __channelCount = 0; // # of channels left. When we hit 0, then remove our handler
28-
static AudioBufferManager* __channelMap[12]; // Lets the IRQ handler figure out where to dispatch to
27+
static int __channelCount = 0; // # of channels left. When we hit 0, then remove our handler
28+
static AudioBufferManager* __channelMap[12]; // Lets the IRQ handler figure out where to dispatch to
29+
static bool __irqInstalled = false; // Have we put in our IRQ handler yet?
2930

3031
AudioBufferManager::AudioBufferManager(size_t bufferCount, size_t bufferWords, int32_t silenceSample, PinMode direction, enum dma_channel_transfer_size dmaSize) {
3132
_running = false;
@@ -78,11 +79,6 @@ AudioBufferManager::~AudioBufferManager() {
7879
dma_channel_unclaim(_channelDMA[i]);
7980
__channelCount--;
8081
}
81-
if (!__channelCount) {
82-
irq_set_enabled(DMA_IRQ_0, false);
83-
// TODO - how can we know if there are no other parts of the core using DMA0 IRQ??
84-
irq_remove_handler(DMA_IRQ_0, _irq);
85-
}
8682
}
8783
interrupts();
8884
for (int i = 0; i < 2; i++) {
@@ -120,7 +116,7 @@ bool AudioBufferManager::begin(int dreq, volatile void *pioFIFOAddr) {
120116
return false;
121117
}
122118
}
123-
bool needSetIRQ = __channelCount == 0;
119+
124120
// Need to know both channels to set up ping-pong, so do in 2 stages
125121
for (auto i = 0; i < 2; i++) {
126122
dma_channel_config c = dma_channel_get_default_config(_channelDMA[i]);
@@ -146,9 +142,10 @@ bool AudioBufferManager::begin(int dreq, volatile void *pioFIFOAddr) {
146142
__channelMap[_channelDMA[i]] = this;
147143
__channelCount++;
148144
}
149-
if (needSetIRQ) {
145+
if (!__irqInstalled) {
150146
irq_add_shared_handler(DMA_IRQ_0, _irq, PICO_SHARED_IRQ_HANDLER_DEFAULT_ORDER_PRIORITY);
151147
irq_set_enabled(DMA_IRQ_0, true);
148+
__irqInstalled = true;
152149
}
153150

154151
dma_channel_start(_channelDMA[0]);

‎libraries/BluetoothAudio/examples/A2DPSink/A2DPSink.ino

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,15 +45,13 @@ void setup() {
4545

4646
void loop() {
4747
if (BOOTSEL) {
48-
__lockBluetooth();
4948
if (status == A2DPSink::PAUSED) {
5049
a2dp.play();
5150
Serial.printf("Resuming\n");
5251
} else if (status == A2DPSink::PLAYING) {
5352
a2dp.pause();
5453
Serial.printf("Pausing\n");
5554
}
56-
__unlockBluetooth();
5755
while (BOOTSEL);
5856
}
5957
}

‎libraries/BluetoothAudio/src/A2DPSink.cpp

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -191,9 +191,10 @@ bool A2DPSink::begin() {
191191
}
192192

193193
bool A2DPSink::disconnect() {
194-
__lockBluetooth();
195-
a2dp_sink_disconnect(a2dp_sink_a2dp_connection.a2dp_cid);
196-
__unlockBluetooth();
194+
BluetoothLock b;
195+
if (_connected) {
196+
a2dp_sink_disconnect(a2dp_sink_a2dp_connection.a2dp_cid);
197+
}
197198
if (!_running || !_connected) {
198199
return false;
199200
}
@@ -202,10 +203,11 @@ bool A2DPSink::disconnect() {
202203
}
203204

204205
void A2DPSink::clearPairing() {
205-
disconnect();
206-
__lockBluetooth();
206+
BluetoothLock b;
207+
if (_connected) {
208+
a2dp_sink_disconnect(a2dp_sink_a2dp_connection.a2dp_cid);
209+
}
207210
gap_delete_all_link_keys();
208-
__unlockBluetooth();
209211
}
210212

211213

‎libraries/BluetoothAudio/src/A2DPSink.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
#include <Arduino.h>
2424
#include "BluetoothHCI.h"
25+
#include "BluetoothLock.h"
2526
#include "BluetoothAudioConsumer.h"
2627
#include "BluetoothMediaConfigurationSBC.h"
2728

@@ -120,60 +121,70 @@ class A2DPSink : public Stream {
120121
void playback_handler(int16_t * buffer, uint16_t num_audio_frames);
121122

122123
void play() {
124+
BluetoothLock b;
123125
if (_connected) {
124126
avrcp_controller_play(a2dp_sink_avrcp_connection.avrcp_cid);
125127
}
126128
}
127129

128130
void stop() {
131+
BluetoothLock b;
129132
if (_connected) {
130133
avrcp_controller_stop(a2dp_sink_avrcp_connection.avrcp_cid);
131134
}
132135
}
133136

134137
void pause() {
138+
BluetoothLock b;
135139
if (_connected) {
136140
avrcp_controller_pause(a2dp_sink_avrcp_connection.avrcp_cid);
137141
}
138142
}
139143

140144
void fastForward() {
145+
BluetoothLock b;
141146
if (_connected) {
142147
avrcp_controller_fast_forward(a2dp_sink_avrcp_connection.avrcp_cid);
143148
}
144149
}
145150

146151
void rewind() {
152+
BluetoothLock b;
147153
if (_connected) {
148154
avrcp_controller_rewind(a2dp_sink_avrcp_connection.avrcp_cid);
149155
}
150156
}
151157

152158
void forward() {
159+
BluetoothLock b;
153160
if (_connected) {
154161
avrcp_controller_forward(a2dp_sink_avrcp_connection.avrcp_cid);
155162
}
156163
}
157164

158165
void backward() {
166+
BluetoothLock b;
159167
if (_connected) {
160168
avrcp_controller_backward(a2dp_sink_avrcp_connection.avrcp_cid);
161169
}
162170
}
163171

164172
void volumeUp() {
173+
BluetoothLock b;
165174
if (_connected) {
166175
avrcp_controller_volume_up(a2dp_sink_avrcp_connection.avrcp_cid);
167176
}
168177
}
169178

170179
void volumeDown() {
180+
BluetoothLock b;
171181
if (_connected) {
172182
avrcp_controller_volume_down(a2dp_sink_avrcp_connection.avrcp_cid);
173183
}
174184
}
175185

176186
void mute() {
187+
BluetoothLock b;
177188
if (_connected) {
178189
avrcp_controller_mute(a2dp_sink_avrcp_connection.avrcp_cid);
179190
}

‎libraries/BluetoothAudio/src/A2DPSource.cpp

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -196,11 +196,11 @@ bool A2DPSource::connect(const uint8_t *addr) {
196196
return false;
197197
}
198198
}
199-
200199
bool A2DPSource::disconnect() {
201-
__lockBluetooth();
202-
a2dp_source_disconnect(media_tracker.a2dp_cid);
203-
__unlockBluetooth();
200+
BluetoothLock b;
201+
if (_connected) {
202+
a2dp_source_disconnect(media_tracker.a2dp_cid);
203+
}
204204
if (!_running || !_connected) {
205205
return false;
206206
}
@@ -209,17 +209,20 @@ bool A2DPSource::disconnect() {
209209
}
210210

211211
void A2DPSource::clearPairing() {
212-
disconnect();
213-
__lockBluetooth();
212+
BluetoothLock b;
213+
if (_connected) {
214+
a2dp_source_disconnect(media_tracker.a2dp_cid);
215+
}
214216
gap_delete_all_link_keys();
215-
__unlockBluetooth();
216217
}
217218

218219
// from Print (see notes on write() methods below)
219220
size_t A2DPSource::write(const uint8_t *buffer, size_t size) {
221+
BluetoothLock b;
222+
220223
size_t count = 0;
221224
size /= 2;
222-
__lockBluetooth();
225+
223226
// First copy from writer to either end of
224227
uint32_t start = _pcmWriter;
225228
uint32_t end = _pcmReader > _pcmWriter ? _pcmReader : _pcmBufferSize;
@@ -244,21 +247,19 @@ size_t A2DPSource::write(const uint8_t *buffer, size_t size) {
244247
_pcmWriter += end - start;
245248
_pcmWriter %= _pcmBufferSize;
246249
}
247-
__unlockBluetooth();
248250
return count;
249251
}
250252

251253
int A2DPSource::availableForWrite() {
254+
BluetoothLock b;
252255
int avail = 0;
253-
__lockBluetooth();
254256
if (_pcmWriter == _pcmReader) {
255257
avail = _pcmBufferSize - 1;
256258
} else if (_pcmReader > _pcmWriter) {
257259
avail = _pcmReader - _pcmWriter - 1;
258260
} else {
259261
avail = _pcmBufferSize - _pcmWriter + _pcmReader - 1;
260262
}
261-
__unlockBluetooth();
262263
return avail;
263264
}
264265

‎libraries/BluetoothAudio/src/A2DPSource.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
#include <Arduino.h>
2424
#include "BluetoothHCI.h"
25+
#include "BluetoothLock.h"
2526
#include "BluetoothMediaConfigurationSBC.h"
2627
#include <functional>
2728
#include <list>
@@ -84,13 +85,12 @@ class A2DPSource : public Stream {
8485
}
8586

8687
bool getUnderflow() {
88+
BluetoothLock b;
8789
if (!_running) {
8890
return false;
8991
}
90-
__lockBluetooth();
9192
auto ret = _underflow;
9293
_underflow = false;
93-
__unlockBluetooth();
9494
return ret;
9595
}
9696

‎libraries/BluetoothAudio/src/BluetoothAudio.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#include "BluetoothLock.h"
12
#include "BluetoothDevice.h"
23
#include "BluetoothHCI.h"
34
#include "BluetoothMediaConfigurationSBC.h"

‎libraries/BluetoothAudio/src/BluetoothHCI.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include <memory>
2525

2626
#include "BluetoothHCI.h"
27+
#include "BluetoothLock.h"
2728

2829
#define CCALLBACKNAME _BTHCICB
2930
#include <ctocppcallback.h>
@@ -46,10 +47,9 @@ void BluetoothHCI_::begin() {
4647
}
4748

4849
void BluetoothHCI_::uninstall() {
49-
__lockBluetooth();
50+
BluetoothLock b;
5051
hci_remove_event_handler(&hci_event_callback_registration);
5152
_running = false;
52-
__unlockBluetooth();
5353
}
5454

5555
bool BluetoothHCI_::running() {
@@ -72,9 +72,11 @@ std::list<BTDeviceInfo> BluetoothHCI_::scan(uint32_t mask, int scanTimeSec, bool
7272
inquiryTime = 1;
7373
}
7474
DEBUGV("HCI::scan(): inquiry start\n");
75-
__lockBluetooth();
76-
gap_inquiry_start(inquiryTime);
77-
__unlockBluetooth();
75+
// Only need to lock around the inquiry start command, not the wait
76+
{
77+
BluetoothLock b;
78+
gap_inquiry_start(inquiryTime);
79+
}
7880
if (async) {
7981
return _btdList;
8082
}

‎libraries/BluetoothAudio/src/BluetoothLock.h

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/*
2+
Bluetooth lock helper class
3+
4+
Copyright (c) 2024 Earle F. Philhower, III <earlephilhower@yahoo.com>
5+
6+
This library is free software; you can redistribute it and/or
7+
modify it under the terms of the GNU Lesser General Public
8+
License as published by the Free Software Foundation; either
9+
version 2.1 of the License, or (at your option) any later version.
10+
11+
This library is distributed in the hope that it will be useful,
12+
but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14+
Lesser General Public License for more details.
15+
16+
You should have received a copy of the GNU Lesser General Public
17+
License along with this library; if not, write to the Free Software
18+
Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
19+
*/
20+
21+
#pragma once
22+
23+
#include <Arduino.h>
24+
25+
class BluetoothLock {
26+
public:
27+
BluetoothLock() {
28+
__lockBluetooth();
29+
}
30+
~BluetoothLock() {
31+
__unlockBluetooth();
32+
}
33+
};

0 commit comments

Comments
 (0)
0