Add sys::furi::Error type for kernel errors

This is an updated attempt at improving the ergonomics around working
with `FuriStatus`. Rather than trying to use `sys::furi::Status` as a
pseudo-`Error`/`Result` type, use a dedicated `Error` enum.

In a future PR I'm going to change how the bindings for `enum FuriStatus`
to a newtype rather than integer constant. This will remove the need
for a seperate `sys::furi::Status` type and the associated hoops required
to turn that into a `Result` type. Being an alias for `i32` limits
what traits or functions can be implemented on the type.

CHANGELOG.md

@@ -14,12 +14,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 - `as_ticks()` method to `flipperzero::furi::time::Duration`
 - `flipperzero::furi::thread::sleep_ticks` function to sleep for exact duration
 - `TryFrom<core::time::Duration>` for `flipperzero::furi::time::Duration`
+- `sys::furi::Error` as error type for Kernel operations.
 
 ### Changed
 
 - `flipperzero::dialogs::DialogFileBrowserOptions` now uses native initialization function.
 - `flipperzero::time::Duration::MAX` is now the maximum duration representable.
-- `sys::furi::Status::err_or_else` now takes a `Fn(i32) -> T` closure.
+- `sys::furi::Status::err_or_else` has been replaced by `sys::furi::Status::into_result`.
 
 ### Removed
 

crates/flipperzero/src/furi/kernel.rs

@@ -49,7 +49,7 @@ impl From<LockState> for i32 {
 pub fn lock() -> furi::Result<LockState> {
     let status = sys::furi::Status::from(unsafe { sys::furi_kernel_lock() });
 
-    status.err_or_else(LockState::from)
+    status.into_result().map(LockState::from)
 }
 
 /// Unlock kernel, resume process scheduling.
@@ -60,7 +60,7 @@ pub fn lock() -> furi::Result<LockState> {
 pub fn unlock() -> furi::Result<LockState> {
     let status = sys::furi::Status::from(unsafe { sys::furi_kernel_unlock() });
 
-    status.err_or_else(LockState::from)
+    Ok(status.into_result()?.into())
 }
 
 /// Restore kernel lock state.
@@ -71,7 +71,7 @@ pub fn unlock() -> furi::Result<LockState> {
 pub fn restore_lock(state: LockState) -> furi::Result<LockState> {
     let status = sys::furi::Status::from(unsafe { sys::furi_kernel_restore_lock(state.into()) });
 
-    status.err_or_else(LockState::from)
+    Ok(status.into_result()?.into())
 }
 
 /// Return kernel tick frequency in hertz.

crates/flipperzero/src/furi/message_queue.rs

@@ -40,7 +40,9 @@ impl<M: Sized> MessageQueue<M> {
             .into()
         };
 
-        status.err_or(())
+        let _ = status.into_result()?;
+
+        Ok(())
     }
 
     // Attempts to read a message from the front of the queue within timeout ticks.
@@ -55,11 +57,9 @@ impl<M: Sized> MessageQueue<M> {
             .into()
         };
 
-        if status.is_ok() {
-            Ok(unsafe { out.assume_init() })
-        } else {
-            Err(status)
-        }
+        let _ = status.into_result()?;
+
+        Ok(unsafe { out.assume_init() })
     }
 
     /// Returns the capacity of the queue.
@@ -102,8 +102,6 @@ impl<M: Sized> Drop for MessageQueue<M> {
 mod tests {
     use super::*;
 
-    use flipperzero_sys::furi::Status;
-
     use super::MessageQueue;
 
     #[test]
@@ -129,7 +127,7 @@ mod tests {
         // Attempting to add another message should time out.
         assert_eq!(
             queue.put(7, Duration::from_millis(1)),
-            Err(Status::ERR_TIMEOUT),
+            Err(furi::Error::TimedOut),
         );
 
         // Removing a message from the queue frees up capacity.

crates/flipperzero/src/furi/mod.rs

@@ -15,4 +15,4 @@ use flipperzero_sys as sys;
 /// Furi Result type.
 pub type Result<T> = core::result::Result<T, Error>;
 /// Furi Error type.
-pub type Error = sys::furi::Status;
+pub type Error = sys::furi::Error;

crates/flipperzero/src/furi/sync.rs

@@ -48,7 +48,8 @@ impl FuriMutex {
     /// `timeout` is zero.
     fn try_acquire(&self, timeout: u32) -> bool {
         let status: Status = unsafe { sys::furi_mutex_acquire(self.get(), timeout).into() };
-        status.is_ok()
+
+        !status.is_err()
     }
 }
 

crates/sys/src/furi.rs

@@ -3,6 +3,88 @@
 use core::ffi::c_char;
 use core::fmt::Display;
 
+use crate::FuriStatus;
+
+/// The error type for Furi kernel operations.
+#[derive(Clone, Copy, Debug, ufmt::derive::uDebug, Eq, PartialEq)]
+pub enum Error {
+    Unspecified,
+    TimedOut,
+    ResourceBusy,
+    InvalidParameter,
+    OutOfMemory,
+    ForbiddenInISR,
+    Other(i32),
+}
+
+impl Error {
+    /// Describe the kind of error.
+    pub fn description(&self) -> &str {
+        match self {
+            Self::Unspecified => "Unspecified RTOS error",
+            Self::TimedOut => "Operation not completed within the timeout period",
+            Self::ResourceBusy => "Resource not available",
+            Self::InvalidParameter => "Parameter error",
+            Self::OutOfMemory => "System is out of memory",
+            Self::ForbiddenInISR => "Not allowed in ISR context",
+            _ => "Unknown",
+        }
+    }
+}
+
+/// Create [`Error`] from [`FuriStatus`].
+impl TryFrom<FuriStatus> for Error {
+    type Error = i32;
+
+    fn try_from(status: crate::FuriStatus) -> core::result::Result<Self, Self::Error> {
+        match status {
+            crate::FuriStatus_FuriStatusError => Ok(Self::Unspecified),
+            crate::FuriStatus_FuriStatusErrorTimeout => Ok(Self::TimedOut),
+            crate::FuriStatus_FuriStatusErrorResource => Ok(Self::ResourceBusy),
+            crate::FuriStatus_FuriStatusErrorParameter => Ok(Self::InvalidParameter),
+            crate::FuriStatus_FuriStatusErrorNoMemory => Ok(Self::OutOfMemory),
+            crate::FuriStatus_FuriStatusErrorISR => Ok(Self::ForbiddenInISR),
+            x => {
+                if x < 0 {
+                    Ok(Self::Other(x))
+                } else {
+                    Err(x)
+                }
+            }
+        }
+    }
+}
+
+/// Create [`FuriStatus`] from [`Error`].
+impl From<Error> for FuriStatus {
+    fn from(error: Error) -> Self {
+        match error {
+            Error::Unspecified => crate::FuriStatus_FuriStatusError,
+            Error::TimedOut => crate::FuriStatus_FuriStatusErrorTimeout,
+            Error::ResourceBusy => crate::FuriStatus_FuriStatusErrorResource,
+            Error::InvalidParameter => crate::FuriStatus_FuriStatusErrorParameter,
+            Error::OutOfMemory => crate::FuriStatus_FuriStatusErrorNoMemory,
+            Error::ForbiddenInISR => crate::FuriStatus_FuriStatusErrorISR,
+            Error::Other(x) => x as crate::FuriStatus,
+        }
+    }
+}
+
+impl Display for Error {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        write!(f, "{} ({})", self.description(), FuriStatus::from(*self))
+    }
+}
+
+impl ufmt::uDisplay for Error {
+    fn fmt<W>(&self, f: &mut ufmt::Formatter<'_, W>) -> Result<(), W::Error>
+    where
+        W: ufmt::uWrite + ?Sized,
+    {
+        ufmt::uwrite!(f, "{} ({})", self.description(), FuriStatus::from(*self))
+    }
+}
+
 /// Operation status.
 /// The Furi API switches between using `enum FuriStatus`, `int32_t` and `uint32_t`.
 /// Since these all use the same bit representation, we can just "cast" the returns to this type.
@@ -40,31 +122,16 @@ impl Status {
         }
     }
 
-    /// Was the operation successful?
-    pub fn is_ok(self) -> bool {
-        self == Self::OK
-    }
-
-    /// Did the operation error?
+    /// Check if this is an error status.
     pub fn is_err(self) -> bool {
         self != Self::OK
     }
 
-    /// Returns `Err(Status)` if [`Status`] is an error, otherwise `Ok(ok)`.
-    pub fn err_or<T>(self, ok: T) -> Result<T, Self> {
-        if self.is_err() {
-            Err(self)
-        } else {
-            Ok(ok)
-        }
-    }
-
-    /// Returns `Err(Status)` if [`Status`] is an error, otherwise `Ok(or_else(Status))`.
-    pub fn err_or_else<T>(self, or_else: impl Fn(i32) -> T) -> Result<T, Self> {
-        if self.is_err() {
-            Err(self)
-        } else {
-            Ok(or_else(self.0))
+    /// Convert into [`Result`] type.
+    pub fn into_result(self) -> Result<i32, Error> {
+        match Error::try_from(self.0) {
+            Err(x) => Ok(x),
+            Ok(err) => Err(err),
         }
     }
 }
@@ -84,12 +151,18 @@ impl ufmt::uDisplay for Status {
     }
 }
 
-impl From<i32> for Status {
-    fn from(code: i32) -> Self {
+impl From<crate::FuriStatus> for Status {
+    fn from(code: FuriStatus) -> Self {
         Status(code)
     }
 }
 
+impl From<Status> for Result<i32, Error> {
+    fn from(status: Status) -> Self {
+        status.into_result()
+    }
+}
+
 /// Low-level wrapper of a record handle.
 pub struct UnsafeRecord<T> {
     name: *const c_char,