Skip to content
This repository has been archived by the owner on Sep 25, 2019. It is now read-only.

The DWM1001 struct can't be created with RTFM (safely) #80

Closed
jamesmunns opened this issue Feb 23, 2019 · 4 comments
Closed

The DWM1001 struct can't be created with RTFM (safely) #80

jamesmunns opened this issue Feb 23, 2019 · 4 comments
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@jamesmunns
Copy link
Collaborator

Creating the DWM1001 struct requires the use of either ::take() or ::new(). RTFM takes both the Peripherals and CorePeripherals, and gives the user of RTFM a subset of these items (because it takes things like the SysTick timer).

In my template examples, I realized that I never used the DWM1001 struct. I stumbled on this again during a stream - (about 40 minutes in).

I'm not sure the best way to fix this yet, but might take a deeper peek later.

@hannobraun
Copy link
Owner

Oh man, that really sucks! This means that RTFM is incompatible with any crate taking ownership of CorePeriphals/Peripherals, like my lpc82x-hal crate.

This is bad, in my opinion, because I think the whole extension trait pattern the other HALs have going on is deeply flawed. I wrote about that earlier. I've been planning to write a more detailed article about this issue. I need to prioritize this.

Unless there's some easy way out that I can't see right now, either someone needs to prove me wrong, or RTFM needs to be fixed. Otherwise we'll get to choose between sup-par static guarantees in HAL APIs and a split ecosystem (although, to be honest, I seem to be the only one who's split from the rest of the ecosystem right now :-) ).

@hannobraun
Copy link
Owner

I'm mostly through your stream now. I think your particular problem can be solved, or at least alleviated, by making the constructors of Led, DW_RST and other such structs public. Then you could construct your own versions of those from what RTFM gives you.

The more general problem I talked about in my previous comment remains. Pretty sure I couldn't do this in lpc82x-hal without significantly weakening its static guarantees.

@hannobraun
Copy link
Owner

@jamesmunns, @japaric (don't want to ping him needlessly), and I talked about this topic on IRC yesterday. Since RTFM only actually requires CorePeripherals, and handles Peripherals just as a convenience to the user, the broader problem of RTFM clashing with all libraries that take Peripherals can be solved in RTFM. See https://github.com/japaric/cortex-m-rtfm/issues/163.

The more specific problem in this library can be solved by the following action items:

  1. Make constructors of Led, DW_RST, and similiar structs public, as previously mentioned.
  2. Remove anything related to CorePeripherals from DWM1001, or somehow make it optional. I'm not sure right now whether having CorePeripherals is just a convenience or actually required. If it's a convenience, it would be nice to make it optional, if that wouldn't be too complex, but removing it would be fine. If it's required, a solution needs to be found.
  3. Make constructor of DWM1001 public too. I'm have this as a separate item, since the constructor may need to change, depending on how item 2. shakes out.

@hannobraun hannobraun added enhancement New feature or request help wanted Extra attention is needed good first issue Good for newcomers labels Mar 6, 2019
@hannobraun
Copy link
Owner

I'm going to close this. The immediate problem was fixed in #82. The more general problem is being discussed in https://github.com/japaric/cortex-m-rtfm/issues/163.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants