fw: clean up and get command processing actually working

Still an issue where connecting to a socket the first time will time
out, unsure what the cause of that or where to start, but can ignore for
now and figure out more when I have more info later

firmware/Cargo.toml

@@ -20,3 +20,4 @@ features = ["medium-ethernet", "proto-ipv4", "socket-tcp", "socket-icmp", "defmt
 
 [profile.release]
 debug = true
+opt-level = "s"

firmware/build_and_strip.sh

@@ -1,5 +1,5 @@
 #!/usr/bin/sh
-DEFMT_LOG=trace cargo build --release
+DEFMT_LOG=debug cargo build --release
 if [ $? -ne 0 ]
 then
     exit $?

firmware/memory.x

@@ -1,7 +1,7 @@
 MEMORY
 {
   RAM : ORIGIN = 0x10000000, LENGTH = 64K
-  FLASH : ORIGIN = 0x00000000, LENGTH = 80K
+  FLASH : ORIGIN = 0x00000000, LENGTH = 100K
 }
 
 REGION_ALIAS("REGION_TEXT", FLASH);

firmware/src/command_interface.rs

@@ -8,6 +8,8 @@ pub struct CommandInterface {
     parser: PacketParser,
 }
 
+const COMMAND_PORT: u16 = 2000;
+
 impl CommandInterface {
     pub fn new() -> Self {
         Self {
@@ -22,8 +24,16 @@ impl CommandInterface {
         // provides a buffer of length 0 when there is nothing to receive, so we
         // are good there. We just reset on any smoltcp errors.
 
-        // TODO should put packet parsing behind a fn with the smoltcp result, then on smoltcp error
+        if !sock.is_open() {
-        // we reconfigure here instead of calling it 4 times
+            defmt::debug!("Socket has been closed");
+            // Socket has been closed, some error has occured
+            sock.listen(COMMAND_PORT);
+            return;
+        }
+
+        if !sock.can_recv() {
+            return;
+        }
 
         // Try and parse a packet
         let res = sock.recv(|rx_buf| {
@@ -34,23 +44,27 @@ impl CommandInterface {
 
         // Check for socket errors
         let packet = match res {
+            // We got a packet! unwrap it
             Ok(Some(packet)) => packet,
-            // No packet to process
+            // No packet to process, move on
             Ok(None) => return,
-            // Any socket error means we just want to drop the connection and re-connect
             Err(_) => {
-                Self::reestablish_socket(sock);
+                defmt::debug!("rx err");
+                sock.abort();
                 return;
             }
         };
 
+        defmt::debug!("Packet rx");
+
         // Check that setting is valid
         let setting = match Settings::try_from(packet.setting) {
             Ok(v) => v,
             Err(()) => {
-                // Write out an error packet
+                // Write out an error packet, we'll just ignore errors
-                let result = sock.send(|tx_buf| {
+                let _ = sock.send(|tx_buf| {
                     if tx_buf.len() < 8 {
+                        // Just drop the packet if we don't have buffer available
                         return (0, ());
                     }
 
@@ -59,20 +73,17 @@ impl CommandInterface {
                     return (8, ());
                 });
 
-                if let Err(_) = result {
-                    // TX error, close socket and reconnect
-                    Self::reestablish_socket(sock);
-                }
-
                 return;
             }
         };
 
+        defmt::debug!("Valid packet: {:?}, is_write: {}, value: {}", packet.setting, packet.is_write, packet.value);
+
         // TODO validate setting values
 
         // TODO handle actually changing/getting settings in all the ways
         // For temp testing, return values sent
-        match sock.send(|tx_buf| {
+        let res = sock.send(|tx_buf| {
             if tx_buf.len() < 8 {
                 // Since this is a low-BW configuration socket, we assume we have space, and drop
                 // the response otherwise. If this is an issue, may re-think this and queue commands but
@@ -82,16 +93,11 @@ impl CommandInterface {
             let response = serialize_response_value(setting, packet.value);
             &tx_buf[0..8].copy_from_slice(&response);
             return (8, ());
-        }) {
+        });
-            Ok(()) => (),
-            Err(_) => {
-                Self::reestablish_socket(sock);
-            }
-        };
-    }
 
-    fn reestablish_socket(sock: &mut Socket) {
+        if res.is_err() {
-        sock.abort();
+            defmt::debug!("tx err");
-        sock.listen(2000);
+            sock.abort();
+        }
     }
 }

firmware/src/eth.rs

@@ -38,9 +38,6 @@ const SLOT_LEN: u32 = 2048;
 
 use crate::{busy_wait, read_reg, write_reg};
 
-use crate::uart::AmlibUart;
-use core::fmt::Write;
-
 pub struct LiteEthDevice {
     csr_addr: u32,
     ethmac_addr: u32,

firmware/src/litex_uart.rs

@@ -1,45 +0,0 @@
-//! Quick and dirty LiteX uart drier
-
-
-const REG_RXTX: u32 = 0;
-const REG_TXFULL: u32 = 0x4;
-//const REG_RXEMPTY: u32 = 0x8;
-
-use crate::{write_reg, read_reg};
-use core::fmt::Write;
-
-pub enum Error {
-    TxFull,
-    RxEmpty,
-}
-
-pub struct LiteXUart {
-    base_addr: u32,
-}
-
-impl LiteXUart {
-    pub fn new(base_addr: u32) -> Self{ Self {base_addr} }
-
-    pub fn try_put_char(&mut self, c: u8) -> Result<(), Error> {
-        unsafe {
-            if read_reg::<u32>(self.base_addr + REG_TXFULL) != 0 {
-                return Err(Error::TxFull);
-            }
-
-            write_reg::<u32>(self.base_addr + REG_RXTX, c as u32);
-            Ok(())
-        }
-    }
-}
-
-impl Write for LiteXUart {
-    fn write_str(&mut self, s: &str) -> core::fmt::Result {
-        for b in s.as_bytes() {
-            // It's okay to loop on this because we'll always clear the buffer
-            while let Err(Error::TxFull) = self.try_put_char(*b) {}
-            //self.try_put_char(*b);
-        }
-
-        Ok(())
-    }
-}

firmware/src/logging.rs

@@ -2,12 +2,14 @@ use core::{fmt::Write, any::Any};
 
 use defmt;
 
-use crate::uart::AmlibUart;
+use crate::uart::LitexUart;
 use core::arch::asm;
 
 #[defmt::global_logger]
 struct DefmtLogger;
 
+static mut LOGGER_SOCKET: Option<&'static mut smoltcp::socket::tcp::Socket> = None;
+
 unsafe impl defmt::Logger for DefmtLogger {
     fn acquire() {
         // Sync methods left empty because we don't use any interrupts
@@ -22,17 +24,24 @@ unsafe impl defmt::Logger for DefmtLogger {
     }
 
     unsafe fn write(bytes: &[u8]) {
-        //static mut UART: Option<AmlibUart> = None;
+        static mut UART: Option<LitexUart> = None;
-        //if UART.is_none() {
+        if UART.is_none() {
-        //    UART = Some(AmlibUart::new(0x0200_0040));
+            UART = Some(LitexUart::new(0xf000_4000));
-        //}
+        }
 
 
-        //let mut dev = UART.unwrap();
+        let mut dev = UART.unwrap();
-        ////writeln!(dev, "a").unwrap();
+        for byte in bytes {
-        ////writeln!(dev, "length: {}", bytes.len());
+            while let Err(_) = dev.try_put_char(*byte) {}
-        //for byte in bytes {
+        }
-        //    while let Err(_) = dev.try_put_char(*byte) {}
-        //}
     }
 }
+
+/// Set the TCP socket to use for defmt logging.
+///
+/// SAFETY: Upcasts the reference from immutable ref to 'static mutable ref,
+/// ensure this socket exists forever, or at least until you call this with a
+/// None.
+pub unsafe fn set_logger_socket(socket: Option<&smoltcp::socket::tcp::Socket>) {
+    LOGGER_SOCKET = core::mem::transmute(socket);
+}

firmware/src/main.rs

@@ -16,6 +16,7 @@ use embedded_hal::prelude::{_embedded_hal_blocking_i2c_Read, _embedded_hal_block
 use mcp4726::Status;
 use riscv_rt::entry;
 use smoltcp::socket::{Socket, self};
+use smoltcp::time::Duration;
 use smoltcp::wire::{IpAddress, Ipv4Address};
 use smoltcp::{
     iface::{SocketSet, SocketStorage},
@@ -29,7 +30,6 @@ mod eth;
 mod i2c;
 mod mcp4726;
 mod uart;
-mod litex_uart;
 mod logging;
 mod proto;
 mod command_interface;
@@ -38,23 +38,6 @@ const MAC: [u8; 6] = [0xA0, 0xBB, 0xCC, 0xDD, 0xEE, 0xF0];
 
 static mut SECONDS: u32 = 0;
 
-/// External interrupt handler
-#[export_name = "MachineExternal"]
-fn external_interrupt_handler() {
-    let cause = riscv::register::mcause::read();
-    let mut uart = litex_uart::LiteXUart::new(0xf000_4000);
-    writeln!(uart, "mcause: {}", cause.bits());
-
-    if (cause.is_interrupt()) {
-        let mut uart = litex_uart::LiteXUart::new(0xf000_4000);
-        writeln!(uart, "mcause: {}", cause.code());
-
-        if cause.code() == 1 {
-            // TIMER0 event, we have reset so count another second
-        }
-    }
-}
-
 // use `main` as the entry point of this application
 // `main` is not allowed to return
 #[entry]
@@ -69,13 +52,10 @@ fn main() -> ! {
     //    }
     //};
     let blink_period = 10_000_000u32;
-    let mut uart = litex_uart::LiteXUart::new(0xf000_4000);
-    writeln!(uart, "uart init");
 
     // enable timer
 
     let mut device = unsafe { eth::LiteEthDevice::try_init(0xf000_0800, 0x8000_0000).unwrap() };
-    writeln!(uart, "eth init");
 
     use smoltcp::wire::{EthernetAddress, HardwareAddress};
     let mut config = smoltcp::iface::Config::default();
@@ -101,11 +81,27 @@ fn main() -> ! {
     let mut socket_storage = [SocketStorage::EMPTY; 4];
     let mut socket_set = SocketSet::new(&mut socket_storage[..]);
 
-    let mut tx_storage = [0u8; 128];
+    let mut tx_storage = [0u8; 64];
-    let mut rx_storage = [0u8; 128];
+    let mut rx_storage = [0u8; 64];
     let mut tx_buf = SocketBuffer::new(&mut tx_storage[..]);
     let mut rx_buf = SocketBuffer::new(&mut rx_storage[..]);
     let mut command_socket = socket_set.add(TcpSocket::new(tx_buf, rx_buf));
+    // Set a keepalive on the socket to handle unexpected client disconnects
+    {
+        let mut sock = socket_set.get_mut::<TcpSocket>(command_socket);
+        sock.set_keep_alive(Some(Duration::from_secs(5)));
+        sock.set_timeout(Some(Duration::from_secs(5)))
+    }
+
+
+    let mut logger_tx_storage = [0u8; 128];
+    let mut logger_rx_storage = [0u8; 16];
+    let mut logger_tx_buf = SocketBuffer::new(&mut logger_tx_storage[..]);
+    let mut logger_rx_buf = SocketBuffer::new(&mut logger_rx_storage[..]);
+    let mut logger_socket = socket_set.add(TcpSocket::new(logger_tx_buf, logger_rx_buf));
+
+    unsafe { logging::set_logger_socket(Some(socket_set.get_mut::<TcpSocket>(logger_socket))); }
+
 
 
     let mut last_blink: u32 = 0;
@@ -134,26 +130,74 @@ fn main() -> ! {
 
     loop {
         let now = millis();
-        if now - last_blink > 1000 {
+        // TODO the need for the second check screams something is unsound somewhere
+        if now - last_blink > 1000 && now > last_blink {
             last_blink = now;
             toggle = !toggle;
             write_led(if toggle { 1 } else { 0 });
 
             let val: u32 = unsafe {read_reg(0x8000_2000)};
-            writeln!(uart, "Sampler value: 0x{:08x}", val).unwrap();
-        }
-
-        if iface.poll(Instant::from_millis(now), &mut device, &mut socket_set) {
-            cmd.run(socket_set.get_mut(command_socket));
         }
 
+        // TODO I think the timer might actually stop until the event is cleared? this may pose
+        // problems, might explain why moving this above the smoltcp stuff "broke" things
         handle_timer_event();
+
+        iface.poll(Instant::from_millis(now), &mut device, &mut socket_set);
+
+        // TODO first connection to a socket takes a while to establish, and can time out, why?
+        cmd.run(socket_set.get_mut(command_socket));
+
+        //if iface.poll(Instant::from_millis(now), &mut device, &mut socket_set) {
+
+            //let sock = socket_set.get_mut::<TcpSocket>(command_socket);
+
+            //if !sock.is_open() {
+            //    sock.listen(2000);
+            //}
+
+            //let mut echo_tx_buf = [0u8; 32];
+            //match sock.recv(|buf| {
+            //    if buf.len() > 0 {
+            //        // Copy into send buffer
+            //        let rd_len = core::cmp::min(buf.len(), echo_tx_buf.len());
+            //        &echo_tx_buf[..rd_len].copy_from_slice(&buf[..rd_len]);
+            //        defmt::trace!("RX data command");
+
+            //        // Return slice to send
+            //        (rd_len, Some(&echo_tx_buf[..rd_len]))
+            //    } else {
+            //        (0, None)
+            //    }
+            //}) {
+            //    Err(_) => {
+            //        // Close the socket, re-open it later
+            //        //sock.abort();
+            //        // Is doing this immediately legal?
+            //        //sock.listen(2000);
+            //    }
+            //    // Some data to send
+            //    Ok(Some(tx_data)) => {
+            //        match sock.send_slice(tx_data) {
+            //            Err(_) => {
+            //                //sock.abort();
+            //                //sock.listen(2000);
+            //            }
+            //            _ => (),
+            //        }
+            //    }
+            //    // No data to send
+            //    Ok(None) => (),
+            //}
+
+        //}
     }
 }
 
 fn handle_timer_event() {
     unsafe {
         if read_reg::<u32>(0xf000_3818) == 0 {
+            // No event yet, continue
             return;
         }
 

firmware/src/uart.rs

@@ -2,18 +2,13 @@
 
 use core::fmt::Write;
 /// TODO repr(C) a register bank, and add instances
-///
 use core::ptr::{read_volatile, write_volatile};
 
-// TODO these offsets may be wrong. I'm still unsure about CSR semantics
+const REG_RXTX: u32 = 0x00;
-const REG_DIVISOR_OFFSET: u32 = 0;
+const REG_TXFULL: u32 = 0x04;
-const REG_SR_OFFSET: u32 = 2;
+const REG_RXEMPTY: u32 = 0x08;
-const REG_DR_OFFSET: u32 = 3;
+const REG_TXEMPTY: u32 = 0x18;
-
+const REG_RXFULL: u32 = 0x1c;
-const FLAG_SR_TX_FULL: u8 = 1 << 0;
-const FLAG_SR_TX_EMPTY: u8 = 1 << 1;
-const FLAG_SR_RX_FULL: u8 = 1 << 2;
-const FLAG_SR_RX_EMPTY: u8 = 1 << 3;
 
 pub enum Error {
     TxFull,
@@ -22,41 +17,43 @@ pub enum Error {
 
 // Hacky derive, shouldn't exist but it's fine :tm:
 #[derive(Copy, Clone)]
-pub struct AmlibUart {
+pub struct LitexUart {
     base_addr: u32,
 }
 
-impl AmlibUart {
+impl LitexUart {
     pub fn new(base_addr: u32) -> Self {
         Self { base_addr }
     }
 
-    pub fn read_status(&mut self) -> u8 {
+    unsafe fn read_reg(&mut self, offset: u32) -> u32 {
-        unsafe { read_volatile((self.base_addr + REG_SR_OFFSET) as *const u8) }
+        read_volatile((self.base_addr + offset) as *const u32)
     }
 
     pub fn try_get_char(&mut self) -> Result<u8, Error> {
-        if self.read_status() & FLAG_SR_RX_EMPTY != 0 {
+        unsafe {
-            return Err(Error::RxEmpty);
+            if self.read_reg(REG_RXEMPTY) != 0 {
-        }
+                return Err(Error::RxEmpty);
+            }
 
-        unsafe { Ok(read_volatile((self.base_addr + REG_DR_OFFSET) as *const u8)) }
+            Ok(read_volatile((self.base_addr + REG_RXTX) as *mut u8))
+        }
     }
 
     pub fn try_put_char(&mut self, c: u8) -> Result<(), Error> {
-        if self.read_status() & FLAG_SR_TX_FULL != 0 {
-            return Err(Error::TxFull);
-        }
-
         unsafe {
-            write_volatile((self.base_addr + REG_DR_OFFSET) as *mut u8, c);
+            if self.read_reg(REG_TXFULL) != 0 {
+                return Err(Error::TxFull);
+            }
+
+            write_volatile((self.base_addr + REG_RXTX) as *mut u8, c);
+            Ok(())
         }
-        Ok(())
     }
 }
 
 // Blocking implementation of write
-impl Write for AmlibUart {
+impl Write for LitexUart {
     fn write_str(&mut self, s: &str) -> core::fmt::Result {
         for b in s.as_bytes() {
             // It's okay to loop on this because we'll always clear the buffer