golemparts/rppal

Duplicating embedded-hal API

hargoniX opened this issue · 1 comments

hargoniX commented

I was taking a look at your SPI documentation and there i see:

pub fn read(&mut self, buffer: &mut [u8]) -> Result<usize>
pub fn write(&mut self, buffer: &[u8]) -> Result<usize>
pub fn transfer(
    &self,
    read_buffer: &mut [u8],
    write_buffer: &[u8]
) -> Result<usize>

But you also implement the embedded-hal traits which in turn provide

fn write(&mut self, words: &[W]) -> Result<(), Self::Error>
fn transfer<'w>(&mut self, words: &'w mut [W]) -> Result<&'w [W], Self::Error>
fn read(&mut self) -> Result<Word, Self::Error>
fn send(&mut self, word: Word) -> Result<(), Self::Error>

Which also provide a read, write and transfer functionality.

So I'm wondering, is there any reason you're essentially duplicating the embedded-hal API here while also at the same time implementing the embedded-hal traits? Wouldn't it make more sense to just implement the embedded-hal traits like the vast majority of HALs (if not all of them) do?

golemparts commented

Thanks for opening this issue. I apologize for the late response. I haven't had a chance to invest more time in this project until recently.

RPPAL predates embedded-hal. Support for embedded-hal was added after RPPAL's SPI API was already defined and in use. Changing RPPAL's API to duplicate the API of an optional feature is not something I considered at the time, since it would break existing projects with no immediate benefits, and developers who prefer the embedded-hal API can simply choose to use that instead of the standard API.

I'll be going through the entire API before 1.0, so it's possible things may change, but there would have to be a clear benefit to any changes made. For now, copying embedded-hal isn't planned.

  • 👎1