micro-ROS / system_modes

System modes for ROS 2 and micro-ROS
Apache License 2.0
43 stars 9 forks source link

More flexibility in specifying the default mode #84

Closed norro closed 3 years ago

norro commented 3 years ago

This PR changes how default modes for parts (systems and nodes) are specified. The previous way of doing so, naming one of the modes __DEFAULT__ is now deprecated, but still possible for backward compatibility.

Instead, each part now specifies an additional defaultmode argument in the SMH file as documented here and exemplified here.

codecov[bot] commented 3 years ago

Codecov Report

Merging #84 (7958be5) into master (5d13a21) will decrease coverage by 0.01%. The diff coverage is 35.92%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #84      +/-   ##
==========================================
- Coverage   32.34%   32.33%   -0.02%     
==========================================
  Files          14       14              
  Lines        1048     1104      +56     
  Branches      754      787      +33     
==========================================
+ Hits          339      357      +18     
- Misses        129      132       +3     
- Partials      580      615      +35     
Impacted Files Coverage Δ
system_modes/include/system_modes/mode.hpp 100.00% <ø> (ø)
...stem_modes/include/system_modes/mode_inference.hpp 100.00% <ø> (ø)
system_modes/src/system_modes/mode_manager.cpp 16.72% <14.28%> (-0.57%) :arrow_down:
system_modes/src/system_modes/mode_inference.cpp 36.66% <35.95%> (-0.40%) :arrow_down:
system_modes/src/system_modes/mode.cpp 57.14% <50.00%> (+1.40%) :arrow_up:
system_modes/include/system_modes/mode_impl.hpp 88.00% <100.00%> (+2.81%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 5d13a21...7958be5. Read the comment docs.

norro commented 3 years ago

@nielsvd Please give this PR a review. It changes the system modes packages so that you can now declare arbitrary modes to be the default mode.

Sorry for the huge PR, I did some necessary changes that you don't have to review IMHO, since parsing and inference are quite complex by now (I started refactoring in a separate branch, which should make it more readable soon).

For your review I would like you to notice that I only had to change the mode names in the examples and launch tests, no further changes (from a system modes user's perspective) were necessary. So I expect that it is similar in your use case, you may just drop the __DEFAULT__ modes and mark arbitrary other modes of your model with defaultmode: true