Skip to content
This repository was archived by the owner on Jul 6, 2019. It is now read-only.

Fix platform tree for Tiva C/Stellaris Launchpad boards#399

Merged
farcaller merged 6 commits intohackndev:masterfrom
jamwaffles:tiva-c-regression-fix
Oct 11, 2016
Merged

Fix platform tree for Tiva C/Stellaris Launchpad boards#399
farcaller merged 6 commits intohackndev:masterfrom
jamwaffles:tiva-c-regression-fix

Conversation

@jamwaffles
Copy link
Contributor

This pull request fixes regressions in the mcu_tiva_c platform tree allowing Zinc to be built for the Stellaris Launchpad and Tiva C boards. I've test-compiled a boilerplate project onto my Stellaris Launchpad which works, and I've compiled a few of the tiva_c_* projects under examples/, which built fine as well, although I haven't flashed them to any boards yet.

Apologies for the messy commits. I hope the overall diff makes apparent what I did. There's a lot of .unwrap() unsafe code in the platform tree files, but that seems to be on par with the lpc17xx files, so I left my changes as is. An incorrect platform tree will panic with no error currently, but that seems to be a general issue with Zinc at the moment as the codebase is still in flux.

Notable changes:

  • Closures like |&: err: &str | no longer need &:; the compiler infers this part
  • String::as_slice() doesn't exist any more, so I converted those occurrences to String::as_str()
  • I've commented out #![deny(missing_docs)] in src/lib.rs as it fails builds. Obviously documentation is a good thing, but I feel it's outside the scope of this PR which is just to get the Tiva C build working again.

Software versions:

macOS Sierra

$ rustc --version
rustc 1.13.0-nightly (6ffdda1ba 2016-09-14)

$ arm-none-eabi-ld --version
GNU ld (GNU Tools for ARM Embedded Processors) 2.24.0.20150921

Thank you to the Zinc team for making it possible to write Rust on ARM CPUs!

This is fixed in the next commit. I missed the fact that I was including
the regex crate in my child project, which was trying to compile for ARM
no_std which obviously doesn't work.
@coveralls
Copy link

Coverage Status

Coverage decreased (-6.5%) to 85.657% when pulling a543993 on jamwaffles:tiva-c-regression-fix into 0b38933 on hackndev:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-6.5%) to 85.657% when pulling a543993 on jamwaffles:tiva-c-regression-fix into 0b38933 on hackndev:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-6.5%) to 85.657% when pulling a543993 on jamwaffles:tiva-c-regression-fix into 0b38933 on hackndev:master.

@coveralls
Copy link

coveralls commented Oct 10, 2016

Coverage Status

Coverage decreased (-6.5%) to 85.657% when pulling a543993 on jamwaffles:tiva-c-regression-fix into 0b38933 on hackndev:master.

name = "platformtree"
plugin = true

[dependencies]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a dev-dependencies, I believe?

Copy link
Contributor Author

@jamwaffles jamwaffles Oct 11, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That might work for non-release builds, however when I run cargo build --target=thumbv7em-none-eabi --release in my downstream project, I get extern crate regex; can't find crate so I'm inclined to leave it in dependencies so all builds work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted.

// See the License for the specific language governing permissions and
// limitations under the License.

// extern crate regex;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this.

src/lib.rs Outdated
#![allow(improper_ctypes)]
#![feature(const_fn)]
#![deny(missing_docs)]
// #![deny(missing_docs)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does it trigger on? I'd prefer to fix the docs instead.

Copy link
Contributor Author

@jamwaffles jamwaffles Oct 11, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, fixed docs is the best solution here. It fails on most of the const definitions in the tiva_c platform tree code which probably don't need documenting. For example:

pub const PORT_A: *const Port = 0x40004000 as *const Port;
pub const PORT_B: *const Port = 0x40005000 as *const Port;
pub const PORT_C: *const Port = 0x40006000 as *const Port;
...

in /src/hal/tiva_c/pin.rs. I can add #[allow(missing_docs)] everywhere, but it seems very messy:

#[allow(missing_docs)] pub const PORT_A: *const Port = 0x40004000 as *const Port;
#[allow(missing_docs)] pub const PORT_B: *const Port = 0x40005000 as *const Port;
#[allow(missing_docs)] pub const PORT_C: *const Port = 0x40006000 as *const Port;
...

Should I just add #[allow(missing_docs)] and uncomment the line in src/lib.rs or is there a better way?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, please add #[allow(missing_docs)] and a TODO(jamwaffles) to clean it up, although obviously I wouldn't force you to clean it, merely to track who placed it there

@coveralls
Copy link

Coverage Status

Coverage decreased (-6.5%) to 85.657% when pulling 219b5dd on jamwaffles:tiva-c-regression-fix into 0b38933 on hackndev:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-6.5%) to 85.657% when pulling 219b5dd on jamwaffles:tiva-c-regression-fix into 0b38933 on hackndev:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-6.5%) to 85.657% when pulling 219b5dd on jamwaffles:tiva-c-regression-fix into 0b38933 on hackndev:master.

This reinstates disallow(missing_docs) in the HAL lib.rs and instead
moves the allow(missing_docs) exceptions to the individual files in
tiva_c that were breaking the build. Not a perfect solution but better
than either a global or line by line solution IMO.
@coveralls
Copy link

Coverage Status

Coverage decreased (-6.5%) to 85.657% when pulling 831994f on jamwaffles:tiva-c-regression-fix into 0b38933 on hackndev:master.

Copy link
Member

@farcaller farcaller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@farcaller farcaller merged commit a3d393b into hackndev:master Oct 11, 2016
@jamwaffles
Copy link
Contributor Author

Glad to see it merged, thank you! Now to implement SPI on the Tiva...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants