It would be much more convenient for the user to iterate over the events in the Rust idiomatic way (i.e. "for event in events"), instead of indexing into the vector.
This also lets us drop the hard requirement of explicitly dropping the event before reading new events from the buffer again.
Suggested-by: Kent Gibson warthog618@gmail.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- .../rust/libgpiod/examples/gpio_events.rs | 47 +++---- bindings/rust/libgpiod/examples/gpiomon.rs | 5 +- bindings/rust/libgpiod/src/edge_event.rs | 56 +++------ bindings/rust/libgpiod/src/event_buffer.rs | 119 ++++++++++++++---- bindings/rust/libgpiod/src/lib.rs | 2 + bindings/rust/libgpiod/src/line_request.rs | 5 +- bindings/rust/libgpiod/tests/edge_event.rs | 47 ++++--- 7 files changed, 172 insertions(+), 109 deletions(-)
diff --git a/bindings/rust/libgpiod/examples/gpio_events.rs b/bindings/rust/libgpiod/examples/gpio_events.rs index 77a7d2a5faa1..57dd5b4db546 100644 --- a/bindings/rust/libgpiod/examples/gpio_events.rs +++ b/bindings/rust/libgpiod/examples/gpio_events.rs @@ -59,30 +59,31 @@ fn main() -> Result<()> { Ok(true) => (), }
- let count = request.read_edge_events(&mut buffer)?; - if count == 1 { - let event = buffer.event(0)?; - let cloned_event = request::Event::event_clone(&event)?; - - // This is required before reading events again into the buffer. - drop(event); + let events = request.read_edge_events(&mut buffer)?; + for event in events { + let cloned_event = request::Event::event_clone(event)?; + println!( + "line: {} type: {:?}, time: {:?}", + cloned_event.line_offset(), + cloned_event.event_type(), + cloned_event.timestamp() + ); + println!( + "line: {} type: {:?}, time: {:?}", + event.line_offset(), + event.event_type(), + event.timestamp() + ); + }
- let count = request.read_edge_events(&mut buffer)?; - if count == 1 { - let event = buffer.event(0)?; - println!( - "line: {} type: {:?}, time: {:?}", - cloned_event.line_offset(), - cloned_event.event_type(), - cloned_event.timestamp() - ); - println!( - "line: {} type: {:?}, time: {:?}", - event.line_offset(), - event.event_type(), - event.timestamp() - ); - } + let events = request.read_edge_events(&mut buffer)?; + for event in events { + println!( + "line: {} type: {:?}, time: {:?}", + event.line_offset(), + event.event_type(), + event.timestamp() + ); } } } diff --git a/bindings/rust/libgpiod/examples/gpiomon.rs b/bindings/rust/libgpiod/examples/gpiomon.rs index a03ea31dfd53..4dea5d1c8dd7 100644 --- a/bindings/rust/libgpiod/examples/gpiomon.rs +++ b/bindings/rust/libgpiod/examples/gpiomon.rs @@ -58,9 +58,8 @@ fn main() -> Result<()> { Ok(true) => (), }
- let count = request.read_edge_events(&mut buffer)?; - if count == 1 { - let event = buffer.event(0)?; + let events = request.read_edge_events(&mut buffer)?; + for event in events { println!( "line: {} type: {}, time: {:?}", event.line_offset(), diff --git a/bindings/rust/libgpiod/src/edge_event.rs b/bindings/rust/libgpiod/src/edge_event.rs index 95b05e947d26..dc71efcb2396 100644 --- a/bindings/rust/libgpiod/src/edge_event.rs +++ b/bindings/rust/libgpiod/src/edge_event.rs @@ -8,7 +8,6 @@ use std::time::Duration; use super::{ gpiod, line::{EdgeKind, Offset}, - request::Buffer, Error, OperationType, Result, };
@@ -24,29 +23,33 @@ use super::{ /// number of events are being read.
#[derive(Debug, Eq, PartialEq)] -pub struct Event<'b> { - buffer: Option<&'b Buffer>, - event: *mut gpiod::gpiod_edge_event, +pub struct Event { + pub(crate) event: *mut gpiod::gpiod_edge_event, + pub(crate) cloned: bool, }
-impl<'b> Event<'b> { +impl Event { /// Get an event stored in the buffer. - pub(crate) fn new(buffer: &'b Buffer, index: usize) -> Result<Event<'b>> { - // SAFETY: The `gpiod_edge_event` returned by libgpiod is guaranteed to live as long - // as the `struct Event`. - let event = unsafe { - gpiod::gpiod_edge_event_buffer_get_event(buffer.buffer, index.try_into().unwrap()) - }; + pub(crate) fn new(event: *mut gpiod::gpiod_edge_event) -> Event { + Self { + event, + cloned: false, + } + } + + pub fn event_clone(event: &Event) -> Result<Event> { + // SAFETY: `gpiod_edge_event` is guaranteed to be valid here. + let event = unsafe { gpiod::gpiod_edge_event_copy(event.event) }; if event.is_null() { return Err(Error::OperationFailed( - OperationType::EdgeEventBufferGetEvent, + OperationType::EdgeEventCopy, errno::errno(), )); }
Ok(Self { - buffer: Some(buffer), event, + cloned: true, }) }
@@ -95,32 +98,11 @@ impl<'b> Event<'b> { } }
-impl<'e, 'b> Event<'e> { - pub fn event_clone(event: &Event<'b>) -> Result<Event<'e>> - where - 'e: 'b, - { - // SAFETY: `gpiod_edge_event` is guaranteed to be valid here. - let event = unsafe { gpiod::gpiod_edge_event_copy(event.event) }; - if event.is_null() { - return Err(Error::OperationFailed( - OperationType::EdgeEventCopy, - errno::errno(), - )); - } - - Ok(Self { - buffer: None, - event, - }) - } -} - -impl<'b> Drop for Event<'b> { +impl Drop for Event { /// Free the edge event. fn drop(&mut self) { - // Free the event only if a copy is made - if self.buffer.is_none() { + // Only free the cloned event, others are freed with the buffer. + if self.cloned { // SAFETY: `gpiod_edge_event` is guaranteed to be valid here. unsafe { gpiod::gpiod_edge_event_free(self.event) }; } diff --git a/bindings/rust/libgpiod/src/event_buffer.rs b/bindings/rust/libgpiod/src/event_buffer.rs index 16d7022034df..d1bfac54070c 100644 --- a/bindings/rust/libgpiod/src/event_buffer.rs +++ b/bindings/rust/libgpiod/src/event_buffer.rs @@ -4,14 +4,74 @@ // Viresh Kumar viresh.kumar@linaro.org
use std::os::raw::c_ulong; +use std::ptr;
-use super::{gpiod, request, Error, OperationType, Result}; +use super::{ + gpiod, + request::{Event, Request}, + Error, OperationType, Result, +}; + +pub struct Events<'a> { + buffer: &'a mut Buffer, + events: Vec<&'a Event>, + index: usize, +} + +impl<'a> Events<'a> { + pub fn new(buffer: &'a mut Buffer, len: usize) -> Self { + let mut events = Vec::new(); + + for i in 0..len { + // SAFETY: We could have simply pushed the reference of the event to the events vector + // here, but that causes a build error as buffer is borrowed as both mutable and + // immutable. Instead of a reference, perform unsafe pointer magic to do the same + // thing. It is safe as the events buffer will always outlive Events and the reference + // will always be valid. + events.push(unsafe { + (buffer.events.as_ptr().add(i) as *const Event) + .as_ref() + .unwrap() + }); + } + + Self { + buffer, + events, + index: 0, + } + } + + /// Get the number of events the buffer contains. + pub fn len(&self) -> usize { + self.events.len() + } + + /// Check if buffer is empty. + pub fn is_empty(&self) -> bool { + self.events.is_empty() + } +} + +impl<'a> Iterator for Events<'a> { + type Item = &'a Event; + + fn next(&mut self) -> OptionSelf::Item { + if self.events.is_empty() || self.buffer.update_event(self.index).is_err() { + return None; + } + + self.index += 1; + Some(self.events.remove(0)) + } +}
/// Line edge events buffer #[derive(Debug, Eq, PartialEq)] pub struct Buffer { pub(crate) buffer: *mut gpiod::gpiod_edge_event_buffer, - event_count: usize, + events: Vec<Event>, + capacity: usize, }
impl Buffer { @@ -30,22 +90,32 @@ impl Buffer { )); }
+ // SAFETY: `gpiod_edge_event_buffer` is guaranteed to be valid here. + let capacity = unsafe { gpiod::gpiod_edge_event_buffer_get_capacity(buffer) as usize }; + let mut events = Vec::new(); + events.resize_with(capacity, || Event::new(ptr::null_mut())); + Ok(Self { buffer, - event_count: 0, + events, + capacity, }) }
- /// Get a number of edge events from a line request. + /// Get edge events from a line request. /// /// This function will block if no event was queued for the line. - pub fn read_edge_events(&mut self, request: &request::Request) -> Result<u32> { + pub fn read_edge_events(&mut self, request: &Request) -> Result<Events> { + for i in 0..self.events.len() { + self.events[i].event = ptr::null_mut(); + } + // SAFETY: `gpiod_line_request` is guaranteed to be valid here. let ret = unsafe { gpiod::gpiod_line_request_read_edge_event( request.request, self.buffer, - self.capacity().try_into().unwrap(), + self.capacity.try_into().unwrap(), ) };
@@ -55,30 +125,37 @@ impl Buffer { errno::errno(), )) } else { - // Set count of events read in the buffer - self.set_event_count(ret as usize); - Ok(ret as u32) - } - } + let ret = ret as usize;
- /// Set the number of events read into the event buffer. - pub(crate) fn set_event_count(&mut self, count: usize) { - self.event_count = count + if ret > self.capacity { + Err(Error::TooManyEvents(ret, self.capacity)) + } else { + Ok(Events::new(self, ret)) + } + } }
/// Get the capacity of the event buffer. pub fn capacity(&self) -> usize { - // SAFETY: `gpiod_edge_event_buffer` is guaranteed to be valid here. - unsafe { gpiod::gpiod_edge_event_buffer_get_capacity(self.buffer) as usize } + self.capacity }
/// Read an event stored in the buffer. - pub fn event(&self, index: usize) -> Resultrequest::Event { - if index >= self.event_count { - Err(Error::InvalidArguments) - } else { - request::Event::new(self, index) + fn update_event(&mut self, index: usize) -> Result<()> { + // SAFETY: The `gpiod_edge_event` returned by libgpiod is guaranteed to live as long + // as the `struct Event`. + let event = unsafe { + gpiod::gpiod_edge_event_buffer_get_event(self.buffer, index.try_into().unwrap()) + }; + if event.is_null() { + return Err(Error::OperationFailed( + OperationType::EdgeEventBufferGetEvent, + errno::errno(), + )); } + + self.events[index].event = event; + Ok(()) }
/// Get the number of events the buffer contains. diff --git a/bindings/rust/libgpiod/src/lib.rs b/bindings/rust/libgpiod/src/lib.rs index 5452d47d51bc..3bcd3b97647c 100644 --- a/bindings/rust/libgpiod/src/lib.rs +++ b/bindings/rust/libgpiod/src/lib.rs @@ -103,6 +103,8 @@ pub enum Error { OperationFailed(OperationType, errno::Errno), #[error("Invalid Arguments")] InvalidArguments, + #[error("Event count more than buffer capacity: {0} > {1}")] + TooManyEvents(usize, usize), #[error("Std Io Error")] IoError, } diff --git a/bindings/rust/libgpiod/src/line_request.rs b/bindings/rust/libgpiod/src/line_request.rs index 10d2197b876a..7101d098b6f5 100644 --- a/bindings/rust/libgpiod/src/line_request.rs +++ b/bindings/rust/libgpiod/src/line_request.rs @@ -202,7 +202,10 @@ impl Request { /// Get a number of edge events from a line request. /// /// This function will block if no event was queued for the line. - pub fn read_edge_events(&self, buffer: &mut request::Buffer) -> Result<u32> { + pub fn read_edge_events<'a>( + &'a self, + buffer: &'a mut request::Buffer, + ) -> Resultrequest::Events { buffer.read_edge_events(self) } } diff --git a/bindings/rust/libgpiod/tests/edge_event.rs b/bindings/rust/libgpiod/tests/edge_event.rs index dd95f6d82caa..ac32f7b2ba2c 100644 --- a/bindings/rust/libgpiod/tests/edge_event.rs +++ b/bindings/rust/libgpiod/tests/edge_event.rs @@ -93,10 +93,10 @@ mod edge_event { .wait_edge_event(Some(Duration::from_secs(1))) .unwrap());
- assert_eq!(config.request().read_edge_events(&mut buf).unwrap(), 1); - assert_eq!(buf.len(), 1); + let mut events = config.request().read_edge_events(&mut buf).unwrap(); + assert_eq!(events.len(), 1);
- let event = buf.event(0).unwrap(); + let event = events.next().unwrap(); let ts_rising = event.timestamp(); assert_eq!(event.event_type().unwrap(), EdgeKind::Rising); assert_eq!(event.line_offset(), GPIO); @@ -108,10 +108,10 @@ mod edge_event { .wait_edge_event(Some(Duration::from_secs(1))) .unwrap());
- assert_eq!(config.request().read_edge_events(&mut buf).unwrap(), 1); - assert_eq!(buf.len(), 1); + let mut events = config.request().read_edge_events(&mut buf).unwrap(); + assert_eq!(events.len(), 1);
- let event = buf.event(0).unwrap(); + let event = events.next().unwrap(); let ts_falling = event.timestamp(); assert_eq!(event.event_type().unwrap(), EdgeKind::Falling); assert_eq!(event.line_offset(), GPIO); @@ -143,10 +143,10 @@ mod edge_event { .wait_edge_event(Some(Duration::from_secs(1))) .unwrap());
- assert_eq!(config.request().read_edge_events(&mut buf).unwrap(), 1); - assert_eq!(buf.len(), 1); + let mut events = config.request().read_edge_events(&mut buf).unwrap(); + assert_eq!(events.len(), 1);
- let event = buf.event(0).unwrap(); + let event = events.next().unwrap(); assert_eq!(event.event_type().unwrap(), EdgeKind::Rising); assert_eq!(event.line_offset(), GPIO);
@@ -175,10 +175,10 @@ mod edge_event { .wait_edge_event(Some(Duration::from_secs(1))) .unwrap());
- assert_eq!(config.request().read_edge_events(&mut buf).unwrap(), 1); - assert_eq!(buf.len(), 1); + let mut events = config.request().read_edge_events(&mut buf).unwrap(); + assert_eq!(events.len(), 1);
- let event = buf.event(0).unwrap(); + let event = events.next().unwrap(); assert_eq!(event.event_type().unwrap(), EdgeKind::Falling); assert_eq!(event.line_offset(), GPIO);
@@ -207,10 +207,10 @@ mod edge_event { .wait_edge_event(Some(Duration::from_secs(1))) .unwrap());
- assert_eq!(config.request().read_edge_events(&mut buf).unwrap(), 1); - assert_eq!(buf.len(), 1); + let mut events = config.request().read_edge_events(&mut buf).unwrap(); + assert_eq!(events.len(), 1);
- let event = buf.event(0).unwrap(); + let event = events.next().unwrap(); assert_eq!(event.event_type().unwrap(), EdgeKind::Rising); assert_eq!(event.line_offset(), GPIO[0]); assert_eq!(event.global_seqno(), 1); @@ -223,10 +223,10 @@ mod edge_event { .wait_edge_event(Some(Duration::from_secs(1))) .unwrap());
- assert_eq!(config.request().read_edge_events(&mut buf).unwrap(), 1); - assert_eq!(buf.len(), 1); + let mut events = config.request().read_edge_events(&mut buf).unwrap(); + assert_eq!(events.len(), 1);
- let event = buf.event(0).unwrap(); + let event = events.next().unwrap(); assert_eq!(event.event_type().unwrap(), EdgeKind::Rising); assert_eq!(event.line_offset(), GPIO[1]); assert_eq!(event.global_seqno(), 2); @@ -257,15 +257,14 @@ mod edge_event { .wait_edge_event(Some(Duration::from_secs(1))) .unwrap());
- assert_eq!(config.request().read_edge_events(&mut buf).unwrap(), 3); - assert_eq!(buf.len(), 3); + let events = config.request().read_edge_events(&mut buf).unwrap(); + assert_eq!(events.len(), 3);
let mut global_seqno = 1; let mut line_seqno = 1;
// Verify sequence number of events - for i in 0..3 { - let event = buf.event(i).unwrap(); + for event in events { assert_eq!(event.line_offset(), GPIO); assert_eq!(event.global_seqno(), global_seqno); assert_eq!(event.line_seqno(), line_seqno); @@ -293,8 +292,8 @@ mod edge_event { .wait_edge_event(Some(Duration::from_secs(1))) .unwrap());
- assert_eq!(config.request().read_edge_events(&mut buf).unwrap(), 2); - assert_eq!(buf.len(), 2); + let events = config.request().read_edge_events(&mut buf).unwrap(); + assert_eq!(events.len(), 2); } } }