Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review and update documentation #353

Open
BetsyMcPhail opened this issue Jan 15, 2025 · 9 comments
Open

Review and update documentation #353

BetsyMcPhail opened this issue Jan 15, 2025 · 9 comments
Assignees

Comments

@BetsyMcPhail
Copy link
Contributor

The main README needs to be updated to reflect the changes made in #309. Specifically, the advice "Scripts are provided for various CI instances in scripts/continuous_integration." is out-of-date. Other changes or clarifications may be needed as well.

@jwnimmer-tri
Copy link
Contributor

jwnimmer-tri commented Jan 15, 2025

IIRC there was also one small typo recently in one of the docs where the doc said "install_prereqs.sh" but the file was just "install_prereqs" with no extension. It might be worth just doing a top-to-bottom read through and manual test of all examples, as a cleanup pass?

@BetsyMcPhail
Copy link
Contributor Author

That sounds like a good idea to me. @tyler-yankee was going to look into #266, would it make sense for him to do this task first? Both as a fresh pair of eyes and an intro to drake-external-examples.

@jwnimmer-tri
Copy link
Contributor

Yup, sounds good to me. Learning the current examples will help when making a new one.

@tyler-yankee
Copy link
Collaborator

The drake-cmake-installed example points to drake-visualizer as a demo after installing and building drake, but I understand that tool is deprecated. @jwnimmer-tri: thoughts on removing or replacing the reference to drake-visualizer in the README?

@jwnimmer-tri
Copy link
Contributor

jwnimmer-tri commented Jan 16, 2025

The replacement tool would be drake/bin/meldis.

@tyler-yankee
Copy link
Collaborator

@jwnimmer-tri: I'm trying to use meldis but the example in the demo, uniformly_accelerated_particle, is not working. I've noticed some earlier discussions that this example was broken and deleted from drake itself (RobotLocomotion/drake#17653). Is this just an error on my end, or otherwise what should be done with this example?

@jwnimmer-tri
Copy link
Contributor

I'm happy to test it myself, and will let you know. It seems like it only exists in drake_cmake_installed, is that the flavor you're using?

@jwnimmer-tri
Copy link
Contributor

Right, it's the RobotLocomotion/drake#17649 train wreck. You can use this change for now to test:

--- a/drake_cmake_installed/src/particles/uniformly_accelerated_particle.cc
+++ b/drake_cmake_installed/src/particles/uniformly_accelerated_particle.cc
@@ -91,7 +91,7 @@ std::unique_ptr<drake::systems::Context<double>>
 UniformlyAcceleratedParticle::CreateContext(const double position,
                                             const double velocity) const {
   // Allocate context.
-  auto context = this->AllocateContext();
+  auto context = this->CreateDefaultContext();
   // Set continuous state.
   drake::systems::VectorBase<double>& cstate =
       context->get_mutable_continuous_state_vector();

That gets it working for me. (I also ran the example with --target_realtime_rate=0.1 to make it easier to see before it goes off past the horizon.)

I'll figure out the right next step for RobotLocomotion/drake#17649. We might end up deleting the particle from DEE so don't worry about fixing its docs yet.

@BetsyMcPhail
Copy link
Contributor Author

As discussed in the PR #357, there are two follow up tasks: 1. Of all the examples, the only only one that should use /opt is the drake_cmake_installed_apt 2. Use "conditional sudo" calls (#359)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants