From 7b53e8be2a9300b61d41be8b17b2cfed2071e7dd Mon Sep 17 00:00:00 2001 From: David Lenfesty Date: Fri, 16 Jun 2023 14:36:44 -0600 Subject: [PATCH] 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 | 1 + firmware/build_and_strip.sh | 2 +- firmware/memory.x | 2 +- firmware/src/command_interface.rs | 52 ++++++++------- firmware/src/eth.rs | 3 - firmware/src/litex_uart.rs | 45 ------------- firmware/src/logging.rs | 31 +++++---- firmware/src/main.rs | 102 +++++++++++++++++++++--------- firmware/src/uart.rs | 47 +++++++------- 9 files changed, 147 insertions(+), 138 deletions(-) delete mode 100644 firmware/src/litex_uart.rs diff --git a/firmware/Cargo.toml b/firmware/Cargo.toml index 31fe677..5dc050b 100644 --- a/firmware/Cargo.toml +++ b/firmware/Cargo.toml @@ -20,3 +20,4 @@ features = ["medium-ethernet", "proto-ipv4", "socket-tcp", "socket-icmp", "defmt [profile.release] debug = true +opt-level = "s" diff --git a/firmware/build_and_strip.sh b/firmware/build_and_strip.sh index 6f2187d..63e0c7a 100755 --- a/firmware/build_and_strip.sh +++ b/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 $? diff --git a/firmware/memory.x b/firmware/memory.x index 88443fe..576768e 100644 --- a/firmware/memory.x +++ b/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); diff --git a/firmware/src/command_interface.rs b/firmware/src/command_interface.rs index 99199d7..3fa1b67 100644 --- a/firmware/src/command_interface.rs +++ b/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 - // we reconfigure here instead of calling it 4 times + if !sock.is_open() { + 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 - let result = sock.send(|tx_buf| { + // Write out an error packet, we'll just ignore errors + 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) { - sock.abort(); - sock.listen(2000); + if res.is_err() { + defmt::debug!("tx err"); + sock.abort(); + } } } diff --git a/firmware/src/eth.rs b/firmware/src/eth.rs index 292eef1..5c62360 100644 --- a/firmware/src/eth.rs +++ b/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, diff --git a/firmware/src/litex_uart.rs b/firmware/src/litex_uart.rs deleted file mode 100644 index 6596454..0000000 --- a/firmware/src/litex_uart.rs +++ /dev/null @@ -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::(self.base_addr + REG_TXFULL) != 0 { - return Err(Error::TxFull); - } - - write_reg::(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(()) - } -} diff --git a/firmware/src/logging.rs b/firmware/src/logging.rs index e7f33ec..59d2e9a 100644 --- a/firmware/src/logging.rs +++ b/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 = None; - //if UART.is_none() { - // UART = Some(AmlibUart::new(0x0200_0040)); - //} + static mut UART: Option = None; + if UART.is_none() { + UART = Some(LitexUart::new(0xf000_4000)); + } - //let mut dev = UART.unwrap(); - ////writeln!(dev, "a").unwrap(); - ////writeln!(dev, "length: {}", bytes.len()); - //for byte in bytes { - // while let Err(_) = dev.try_put_char(*byte) {} - //} + let mut dev = UART.unwrap(); + 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); +} diff --git a/firmware/src/main.rs b/firmware/src/main.rs index b07b07e..d080e25 100644 --- a/firmware/src/main.rs +++ b/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 rx_storage = [0u8; 128]; + let mut tx_storage = [0u8; 64]; + 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::(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::(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::(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::(0xf000_3818) == 0 { + // No event yet, continue return; } diff --git a/firmware/src/uart.rs b/firmware/src/uart.rs index 4abb7b1..9aae291 100644 --- a/firmware/src/uart.rs +++ b/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_DIVISOR_OFFSET: u32 = 0; -const REG_SR_OFFSET: u32 = 2; -const REG_DR_OFFSET: u32 = 3; - -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; +const REG_RXTX: u32 = 0x00; +const REG_TXFULL: u32 = 0x04; +const REG_RXEMPTY: u32 = 0x08; +const REG_TXEMPTY: u32 = 0x18; +const REG_RXFULL: u32 = 0x1c; 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 { read_volatile((self.base_addr + REG_SR_OFFSET) as *const u8) } + unsafe fn read_reg(&mut self, offset: u32) -> u32 { + read_volatile((self.base_addr + offset) as *const u32) } pub fn try_get_char(&mut self) -> Result { - if self.read_status() & FLAG_SR_RX_EMPTY != 0 { - return Err(Error::RxEmpty); - } + unsafe { + 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