Skip to content

Improve Module 3 exercise#40

Merged
xaru8145 merged 2 commits intomainfrom
fix/workshop_fb
Nov 6, 2025
Merged

Improve Module 3 exercise#40
xaru8145 merged 2 commits intomainfrom
fix/workshop_fb

Conversation

@xaru8145
Copy link
Copy Markdown
Collaborator

@xaru8145 xaru8145 commented Nov 6, 2025

What this PR does

In module 3, avoid misleading constructor test failure when parameter declare and get have intentional typos to force the user to fix node parameter definitions.

3: [==========] Running 7 tests from 2 test suites.
3: [----------] Global test environment set-up.
3: [----------] 4 tests from TestLaserDetectorNodeConstructor
3: [ RUN      ] TestLaserDetectorNodeConstructor.CreatingANodeDoesNotThrow
3: /home/developer/ws/src/module_3/test/test_laser_detector_node.cpp:84: Failure
3: Expected: { const LaserDetectorNode dut(default_node_options_); } doesn't throw an exception.
3:   Actual: it throws rclcpp::exceptions::ParameterNotDeclaredException with description "footprint_radius".
3: 
3: [  FAILED  ] TestLaserDetectorNodeConstructor.CreatingANodeDoesNotThrow (30 ms)
3: [ RUN      ] TestLaserDetectorNodeConstructor.EvaluateConstructorDeclaresParameters
3: unknown file: Failure
3: C++ exception with description "footprint_radius" thrown in the test body.
3: 
3: [  FAILED  ] TestLaserDetectorNodeConstructor.EvaluateConstructorDeclaresParameters (17 ms)
3: [ RUN      ] TestLaserDetectorNodeConstructor.EvaluateConstructorReadsParameters
3: unknown file: Failure
3: C++ exception with description "footprint_radius" thrown in the test body.
3: 
3: [  FAILED  ] TestLaserDetectorNodeConstructor.EvaluateConstructorReadsParameters (17 ms)

To solve this, we define the typo in the same parameter for both declare and get functions, obtaining a meaningful error rather than an uncontrolled exception:

3: [ RUN      ] TestLaserDetectorNodeConstructor.EvaluateConstructorDeclaresParameters
3: [INFO] [1762367206.043342909] [laser_detector_node]: LaserDetectorNode initialized
3: /home/developer/ws/src/module_3/test/test_laser_detector_node.cpp:97: Failure
3: Value of: dut.has_parameter("min_points")
3:   Actual: false
3: Expected: true
3: 
3: [  FAILED  ] TestLaserDetectorNodeConstructor.EvaluateConstructorDeclaresParameters (15 ms)

Also, fix intentional typo in subscriber and place it on the publisher. The test covers the publisher and the test for the subscriber is on the user side. This is a more natural exercise flow.

Type

  • Bugfix
  • Feature
  • Documentation

How to test

Steps to reproduce / test the changes:

  1. Apply solutions to module 2.
  2. Build module 3
  3. Test module 3
  4. Repeat steps 2 and 3 incrementally as you solve the bugs and complete the tests.

Checklist

  • I have signed my commits (git commit -s) or added Signed-off-by to existing commits.
  • I added/updated tests (if applicable)
  • I updated documentation (if applicable)

Related issues

N/A

… a misleading error

Signed-off-by: Xavier Ruiz <xavier.ruiz@ekumenlabs.com>
Signed-off-by: Xavier Ruiz <xavier.ruiz@ekumenlabs.com>
@xaru8145 xaru8145 changed the title Improve Module 3 exercises Improve Module 3 exercise Nov 6, 2025
Copy link
Copy Markdown
Collaborator

@JesusSilvaUtrera JesusSilvaUtrera left a comment

Choose a reason for hiding this comment

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

LGTM, just remeber to adapt the solutions branch acocrdingly, thanks!

@xaru8145 xaru8145 merged commit 399e36b into main Nov 6, 2025
1 of 3 checks passed
@xaru8145 xaru8145 deleted the fix/workshop_fb branch November 6, 2025 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants