Conversation
Reviewer's GuideRefactors the PowerControl module to use Eigen-based RLS, adds error-aware power distribution and additional telemetry accessors, and replaces the STM32/bsp-dev-c-based CI flow with a libxr/Linux-based xrobot test harness while modernizing RLS implementation details. Sequence diagram for omni power limiting with error-aware distributionsequenceDiagram
actor ChassisLoop
participant PowerControl
participant SuperPower
participant Motors3508
ChassisLoop->>PowerControl: SetMotorData3508(output_current, rotorspeed_rpm, speed_error)
ChassisLoop->>PowerControl: CalculatePowerControlParam()
PowerControl->>SuperPower: GetChassisPower()
SuperPower-->>PowerControl: measured_power
PowerControl->>SuperPower: IsOnline()
SuperPower-->>PowerControl: online_flag
PowerControl->>PowerControl: update samples_3508_ and mechanical_power
PowerControl->>PowerControl: compute residual and update params_3508_
ChassisLoop->>PowerControl: OutputLimit(max_power)
alt is_helm_ is false
PowerControl->>PowerControl: OutputLimitOmni(max_power)
loop each 3508 motor
PowerControl->>PowerControl: calculate_motor_model_power()
PowerControl->>PowerControl: accumulate required_power_3508_sum, sum_error_
end
PowerControl->>PowerControl: compute error_confidence_
alt required_power_3508_sum > available_power
loop each 3508 motor
PowerControl->>PowerControl: compute power_weight_error and power_weight_prop
PowerControl->>PowerControl: mix weights using error_confidence_
PowerControl->>PowerControl: solve_current_for_power()
PowerControl-->>Motors3508: new_output_current_3508[i]
end
else power sufficient
PowerControl-->>Motors3508: pass-through output_current_3508
end
else is_helm_ is true
PowerControl->>PowerControl: OutputLimitHelm(max_power)
end
Class diagram for updated PowerControl and RLS modulesclassDiagram
class LibXR_Application
class LibXR_Mutex
class LibXR_HardwareContainer
class LibXR_ApplicationManager
class SuperPower {
+float GetChassisPower()
+float GetCapEnergy()
+bool IsOnline()
}
class PowerControlData {
+float new_output_current_3508[6]
+float new_output_current_6020[6]
+bool is_power_limited
}
class RLS_dim {
+RLS_dim(delta_: float, lambda_: float)
+void Reset()
+ParamVector Update(sample_vector: ParamVector, actual_output: float)
+void SetParamVector(updated_params: ParamVector)
-uint32_t dimension_
-float lambda_
-float delta_
-Matrix_dim_dim transmatrix_
-ParamVector gainvector_
-ParamVector paramsvector_
-ParamVector defaultparamsvector_
}
class PowerControl {
+static int MAX_MOTOR_COUNT
+PowerControl(hw: LibXR_HardwareContainer, app: LibXR_ApplicationManager, superpower: SuperPower, is_helm: bool, chassis_static_power_loss: float, motor_count_3508: int, motor_count_6020: int)
+void SetMotorData3508(output_current: float*, rotorspeed_rpm: float*, speed_error: float*)
+void SetMotorData6020(output_current: float*, rotorspeed_rpm: float*, speed_error: float*)
+void CalculatePowerControlParam()
+void OutputLimit(max_power: float)
+PowerControlData GetPowerControlData()
+float GetMeasuredPower()
+float GetCapEnergy()
+bool IsOnline()
-void OutputLimitOmni(max_power: float)
-void OutputLimitHelm(max_power: float)
-LibXR_Mutex mutex_
-SuperPower* superpower_
-bool is_helm_
-RLS_dim rls_
-PowerControlData powercontrol_data_
-float k3_chassis_
-float error_confidence_
-float sum_error_
-float speed_error_3508_[6]
-float speed_error_6020_[6]
-int motor_count_3508_
-int motor_count_6020_
-float kt_3508_
-float k1_3508_
-float k2_3508_
-Eigen_Matrix_2_1 samples_3508_
-Eigen_Matrix_2_1 params_3508_
-float output_current_3508_[6]
-float rotorspeed_rpm_3508_[6]
-float motor_power_3508_[6]
-float kt_6020_
-float k1_6020_
-float k2_6020_
-float output_current_6020_[6]
-float rotorspeed_rpm_6020_[6]
-float motor_power_6020_[6]
-float measured_power_
}
PowerControl ..|> LibXR_Application
PowerControl o-- SuperPower
PowerControl o-- RLS_dim
PowerControl o-- PowerControlData
PowerControl ..> LibXR_Mutex
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new
solve_current_for_powerimplementation no longer guards againstabeing near-zero ordeltabeing negative and callssqrtf(delta)unconditionally; this can lead to NaNs/division-by-zero and also never updatesfinal_currentin thedelta < 1e-9fcase (it just overwritesoriginal_currentand returns clamped 0), so consider restoring the previous robustness checks and making the small-deltabranch explicitly setfinal_current. - In
RLS::Validateyou still callconfigASSERTbut removed the FreeRTOS headers, and the conditionlambda_ >= 0.0f || lambda_ <= 1.0fis always true; either include the appropriate header or replaceconfigASSERTwith a portable check, and fix the predicate tolambda_ >= 0.0f && lambda_ <= 1.0fso out-of-rangelambdavalues are actually caught.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `solve_current_for_power` implementation no longer guards against `a` being near-zero or `delta` being negative and calls `sqrtf(delta)` unconditionally; this can lead to NaNs/division-by-zero and also never updates `final_current` in the `delta < 1e-9f` case (it just overwrites `original_current` and returns clamped 0), so consider restoring the previous robustness checks and making the small-`delta` branch explicitly set `final_current`.
- In `RLS::Validate` you still call `configASSERT` but removed the FreeRTOS headers, and the condition `lambda_ >= 0.0f || lambda_ <= 1.0f` is always true; either include the appropriate header or replace `configASSERT` with a portable check, and fix the predicate to `lambda_ >= 0.0f && lambda_ <= 1.0f` so out-of-range `lambda` values are actually caught.
## Individual Comments
### Comment 1
<location path=".github/workflows/build.yml" line_range="87-50" />
<code_context>
+ run: |
+ pip3 install xrobot libxr
+
+ - name: Run xrobot_setup
+ run: |
+ xrobot_setup || true
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Using `xrobot_setup || true` may hide real failures in the toolchain setup.
Because later steps rely on a valid XRobot setup, swallowing non-zero exit codes here can turn a clear setup failure into harder-to-debug downstream errors. If there are specific, expected failure modes, handle those explicitly; otherwise, let the step fail so setup issues surface immediately.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| set(XROBOT_MODULES_DIR ${CMAKE_CURRENT_SOURCE_DIR}/Modules/) | ||
| add_subdirectory(libxr) | ||
| target_include_directories(xr_test PUBLIC $<TARGET_PROPERTY:xr,INTERFACE_INCLUDE_DIRECTORIES> ${CMAKE_SOURCE_DIR}/User) | ||
| target_link_libraries(xr_test PUBLIC xr) |
There was a problem hiding this comment.
issue (bug_risk): Using xrobot_setup || true may hide real failures in the toolchain setup.
Because later steps rely on a valid XRobot setup, swallowing non-zero exit codes here can turn a clear setup failure into harder-to-debug downstream errors. If there are specific, expected failure modes, handle those explicitly; otherwise, let the step fail so setup issues surface immediately.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Refine the chassis power control module with Eigen-based RLS estimation, error‑aware power distribution, and updated CI to build and run the module against libxr in a Linux container.
New Features:
Enhancements:
Build: