Skip to content

Commit 2f35daf

Browse files
ArthurHeymansArthurHeymans
committed
fix: move the error handling up the loop
This returns early on problems too which is desirable. Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
1 parent ebf391e commit 2f35daf

File tree

1 file changed

+136
-89
lines changed

1 file changed

+136
-89
lines changed

‎serprog/src/lib.rs‎

Lines changed: 136 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,22 @@ use transport::Transport;
1111
use zerocopy::byteorder::little_endian::{U16, U32};
1212
use zerocopy::{FromBytes, FromZeros, Immutable, IntoBytes, Unaligned};
1313

14-
use defmt::{debug, error};
14+
use defmt::{debug, error, Format};
1515

1616
pub mod transport;
1717

18+
#[derive(Format)]
19+
pub enum SerprogError {
20+
TransportRead(&'static str),
21+
TransportWrite(&'static str),
22+
SpiTransfer(&'static str),
23+
SpiFlush(&'static str),
24+
CsSetLow(&'static str),
25+
CsSetHigh(&'static str),
26+
LedSetLow(&'static str),
27+
LedSetHigh(&'static str),
28+
}
29+
1830
// Convert 3 bytes in little-endian format to u32
1931
fn le_u24_to_u32(bytes: &[u8]) -> u32 {
2032
u32::from_le_bytes([bytes[0], bytes[1], bytes[2], 0])
@@ -213,155 +225,180 @@ where
213225
let mut buf = [0; 1];
214226

215227
loop {
216-
if let Err(e) = self.transport.read(&mut buf).await {
217-
error!("Read error: {:?}", e);
228+
if self.transport.read(&mut buf).await.is_err() {
229+
error!("Read error in main loop");
218230
continue;
219231
}
220232

221233
let cmd = SerprogCommand::try_from(buf[0]).unwrap_or(SerprogCommand::Nop);
222-
self.handle_command(cmd).await;
234+
if let Err(e) = self.handle_command(cmd).await {
235+
error!("Command error: {:?}", e);
236+
}
223237
}
224238
}
225239

226-
async fn handle_command(&mut self, cmd: SerprogCommand)
240+
async fn handle_command(&mut self, cmd: SerprogCommand) -> Result<(), SerprogError>
227241
where
228242
CS::Error: core::fmt::Debug,
229243
LED::Error: core::fmt::Debug,
230244
{
231245
match cmd {
232246
SerprogCommand::Nop => {
233247
debug!("Received Nop CMD");
234-
if let Err(e) = self.transport.write(&[S_ACK]).await {
235-
error!("Error writing packet: {:?}", e);
236-
}
248+
self.transport
249+
.write(&[S_ACK])
250+
.await
251+
.map_err(|_| SerprogError::TransportWrite("Error writing ACK"))?;
252+
Ok(())
237253
}
238254
SerprogCommand::QIface => {
239255
debug!("Received QIface CMD");
240256
let response = QIfaceResponse {
241257
ack: S_ACK,
242258
version: U16::new(1),
243259
};
244-
if let Err(e) = self.transport.write(response.as_bytes()).await {
245-
error!("Error writing packet: {:?}", e);
246-
}
260+
self.transport
261+
.write(response.as_bytes())
262+
.await
263+
.map_err(|_| SerprogError::TransportWrite("Error writing QIface response"))?;
264+
Ok(())
247265
}
248266
SerprogCommand::QCmdMap => {
249267
debug!("Received QCmdMap CMD");
250268
let response = QCmdMapResponse::new(self.freq_callback.is_some());
251-
if let Err(e) = self.transport.write(response.as_bytes()).await {
252-
error!("Error writing packet: {:?}", e);
253-
}
269+
self.transport
270+
.write(response.as_bytes())
271+
.await
272+
.map_err(|_| SerprogError::TransportWrite("Error writing QCmdMap response"))?;
273+
Ok(())
254274
}
255275
SerprogCommand::QPgmName => {
256276
debug!("Received QPgmName CMD");
257277
let response = QPgmNameResponse::new("Picoprog");
258-
if let Err(e) = self.transport.write(response.as_bytes()).await {
259-
error!("Error writing packet: {:?}", e);
260-
}
278+
self.transport
279+
.write(response.as_bytes())
280+
.await
281+
.map_err(|_| SerprogError::TransportWrite("Error writing QPgmName response"))?;
282+
Ok(())
261283
}
262284
SerprogCommand::QSerBuf => {
263285
debug!("Received QSerBuf CMD");
264-
if let Err(e) = self.transport.write(&[S_ACK, 0xFF, 0xFF]).await {
265-
error!("Error writing packet: {:?}", e);
266-
}
286+
self.transport
287+
.write(&[S_ACK, 0xFF, 0xFF])
288+
.await
289+
.map_err(|_| SerprogError::TransportWrite("Error writing QSerBuf response"))?;
290+
Ok(())
267291
}
268292
SerprogCommand::QWrNMaxLen | SerprogCommand::QRdNMaxLen => {
269293
debug!("Received QWrNMaxLen/QRdNMaxLen CMD");
270294
let response = QMaxLenResponse::new(MAX_BUFFER_SIZE);
271-
if let Err(e) = self.transport.write(response.as_bytes()).await {
272-
error!("Error writing packet: {:?}", e);
273-
}
295+
self.transport
296+
.write(response.as_bytes())
297+
.await
298+
.map_err(|_| SerprogError::TransportWrite("Error writing QMaxLen response"))?;
299+
Ok(())
274300
}
275301
SerprogCommand::QBustype => {
276302
debug!("Received QBustype CMD");
277-
if let Err(e) = self.transport.write(&[S_ACK, 0x08]).await {
278-
error!("Error writing packet: {:?}", e);
279-
}
303+
self.transport
304+
.write(&[S_ACK, 0x08])
305+
.await
306+
.map_err(|_| SerprogError::TransportWrite("Error writing QBustype response"))?;
307+
Ok(())
280308
}
281309
SerprogCommand::SyncNop => {
282310
debug!("Received SyncNop CMD");
283-
if let Err(e) = self.transport.write(&[S_NAK, S_ACK]).await {
284-
error!("Error writing packet: {:?}", e);
285-
}
311+
self.transport
312+
.write(&[S_NAK, S_ACK])
313+
.await
314+
.map_err(|_| SerprogError::TransportWrite("Error writing SyncNop response"))?;
315+
Ok(())
286316
}
287317
SerprogCommand::SBustype => {
288318
debug!("Received SBustype CMD");
289319
let mut buf = [0u8; 1];
290-
if let Err(e) = self.transport.read(&mut buf).await {
291-
error!("Error reading packet: {:?}", e);
292-
return;
293-
}
320+
self.transport
321+
.read(&mut buf)
322+
.await
323+
.map_err(|_| SerprogError::TransportRead("Error reading SBustype data"))?;
294324
if buf[0] == 0x08 {
295325
debug!("Received SBustype 'SPI'");
296-
if let Err(e) = self.transport.write(&[S_ACK]).await {
297-
error!("Error writing packet: {:?}", e);
298-
}
326+
self.transport
327+
.write(&[S_ACK])
328+
.await
329+
.map_err(|_| SerprogError::TransportWrite("Error writing SBustype ACK"))?;
299330
} else {
300331
debug!("Received unknown SBustype");
301-
if let Err(e) = self.transport.write(&[S_NAK]).await {
302-
error!("Error writing packet: {:?}", e);
303-
}
332+
self.transport
333+
.write(&[S_NAK])
334+
.await
335+
.map_err(|_| SerprogError::TransportWrite("Error writing SBustype NAK"))?;
304336
}
337+
Ok(())
305338
}
306339
SerprogCommand::OSpiOp => {
307340
debug!("Received OSpiOp CMD");
308341
let mut spi_data = [0_u8; MAX_BUFFER_SIZE * 2 + 6]; // Use twice the size for read and write
309-
if let Err(e) = self.transport.read(&mut spi_data).await {
310-
error!("Error reading packet: {:?}", e);
311-
return;
312-
}
342+
self.transport
343+
.read(&mut spi_data)
344+
.await
345+
.map_err(|_| SerprogError::TransportRead("Error reading OSpiOp data"))?;
313346
let op_slen = le_u24_to_u32(&spi_data[0..3]) as usize;
314347
let op_rlen = le_u24_to_u32(&spi_data[3..6]) as usize;
315348

316349
// This call is blocking according to the SPI HAL
317-
if (self.spi.flush().await).is_err() {
318-
error!("Error flushing SPI");
319-
}
350+
self.spi
351+
.flush()
352+
.await
353+
.map_err(|_| SerprogError::SpiFlush("Error flushing SPI before transfer"))?;
320354

321-
if self.cs.set_low().is_err() {
322-
error!("Error setting CS low");
323-
}
355+
self.cs
356+
.set_low()
357+
.map_err(|_| SerprogError::CsSetLow("Error setting CS low"))?;
324358

325359
let spi_data = &mut spi_data[6..][..op_rlen + op_slen];
326360

327-
match self
328-
.spi
329-
.transfer_in_place(spi_data)
330-
.await
331-
{
361+
match self.spi.transfer_in_place(spi_data).await {
332362
Ok(_) => {
333363
debug!("SPI transfer successful");
334-
if let Err(e) = self.transport.write(&[S_ACK]).await {
335-
error!("Error writing packet: {:?}", e);
336-
}
364+
self.transport.write(&[S_ACK]).await.map_err(|_| {
365+
SerprogError::TransportWrite("Error writing OSpiOp ACK")
366+
})?;
367+
337368
// Embedded HAL says "Implementations are allowed to return before
338369
// the operation is complete" so flush here.
339-
if (self.spi.flush().await).is_err() {
340-
error!("Error flushing SPI");
341-
}
370+
self.spi.flush().await.map_err(|_| {
371+
SerprogError::SpiFlush("Error flushing SPI after transfer")
372+
})?;
373+
342374
let rdata = &spi_data[op_slen..];
343375
// Send the full rdata in chunks
344-
let _ = self.transport.write(rdata).await;
376+
self.transport.write(rdata).await.map_err(|_| {
377+
SerprogError::TransportWrite("Error writing SPI read data")
378+
})?;
345379
}
346380
Err(_) => {
347381
error!("SPI transfer error");
348-
if let Err(e) = self.transport.write(&[S_NAK]).await {
349-
error!("Error writing NAK: {:?}", e);
350-
}
382+
self.transport.write(&[S_NAK]).await.map_err(|_| {
383+
SerprogError::TransportWrite("Error writing OSpiOp NAK")
384+
})?;
385+
return Err(SerprogError::SpiTransfer("SPI transfer failed"));
351386
}
352387
}
353388

354-
if self.cs.set_high().is_err() {
355-
error!("Error setting CS high");
356-
}
389+
self.cs
390+
.set_high()
391+
.map_err(|_| SerprogError::CsSetHigh("Error setting CS high"))?;
392+
393+
Ok(())
357394
}
358395
SerprogCommand::SSpiFreq => {
359396
debug!("Received SSpiFreq CMD");
360397
let mut request = SSpiFreqRequest::new_zeroed();
361-
if let Err(e) = self.transport.read(request.as_mut_bytes()).await {
362-
error!("Error reading packet: {:?}", e);
363-
return;
364-
}
398+
self.transport
399+
.read(request.as_mut_bytes())
400+
.await
401+
.map_err(|_| SerprogError::TransportRead("Error reading SSpiFreq data"))?;
365402

366403
// Parse the request using zerocopy
367404
let try_freq = request.freq.get();
@@ -379,33 +416,43 @@ where
379416
freq: U32::new(try_freq), // TODO can we report what the hardware has set up?
380417
};
381418

382-
if let Err(e) = self.transport.write(response.as_bytes()).await {
383-
error!("Error writing packet: {:?}", e);
384-
}
419+
self.transport
420+
.write(response.as_bytes())
421+
.await
422+
.map_err(|_| SerprogError::TransportWrite("Error writing SSpiFreq response"))?;
423+
424+
Ok(())
385425
}
386426
SerprogCommand::SPinState => {
387427
debug!("Received SPinState CMD");
388428
let mut buf = [0u8; 1];
389-
if let Err(e) = self.transport.read(&mut buf).await {
390-
error!("Error reading packet: {:?}", e);
391-
return;
392-
}
429+
self.transport
430+
.read(&mut buf)
431+
.await
432+
.map_err(|_| SerprogError::TransportRead("Error reading SPinState data"))?;
393433
if buf[0] == 0 {
394-
if self.led.set_low().is_err() {
395-
error!("Error setting LED low");
396-
}
397-
} else if self.led.set_high().is_err() {
398-
error!("Error setting LED high");
399-
}
400-
if let Err(e) = self.transport.write(&[S_ACK]).await {
401-
error!("Error writing packet: {:?}", e);
434+
self.led
435+
.set_low()
436+
.map_err(|_| SerprogError::LedSetLow("Error setting LED low"))?;
437+
} else {
438+
self.led
439+
.set_high()
440+
.map_err(|_| SerprogError::LedSetHigh("Error setting LED high"))?;
402441
}
442+
self.transport
443+
.write(&[S_ACK])
444+
.await
445+
.map_err(|_| SerprogError::TransportWrite("Error writing SPinState ACK"))?;
446+
447+
Ok(())
403448
}
404449
_ => {
405450
debug!("Received unknown CMD");
406-
if let Err(e) = self.transport.write(&[S_NAK]).await {
407-
error!("Error writing packet: {:?}", e);
408-
}
451+
self.transport.write(&[S_NAK]).await.map_err(|_| {
452+
SerprogError::TransportWrite("Error writing unknown command NAK")
453+
})?;
454+
455+
Ok(())
409456
}
410457
}
411458
}

0 commit comments

Comments
 (0)