Conversation
bracyw
left a comment
There was a problem hiding this comment.
Some changes I would like to make before merging
| constructor() { | ||
| // React to defaultValue changes (e.g. from "Set ALL Maps" selector) | ||
| effect(() => { | ||
| const val = this.defaultValue(); |
There was a problem hiding this comment.
This is unneeded we should just put the correct default value in the first time. I believe initial problem was with synchronizing set all and single selectors.
| style="width: 100%; height: 100%" | ||
| [title]="this.getHeatmapTitle()" | ||
| svgIcon="battery_charging_2" | ||
| [slicedLeftCorner]="true" |
There was a problem hiding this comment.
add option for percentage that the corner is sliced.
| ] | ||
| }) | ||
| export class BmsSegmentViewComponent implements OnInit { | ||
| export class BmsSegmentViewComponent implements OnInit, OnDestroy { |
| ></cell-tile> | ||
| } | ||
|
|
||
| @for (cell of betaCells.slice().reverse(); track cell) { |
There was a problem hiding this comment.
add comment saying that reverse is to better match the actual battery setup.
| } | ||
|
|
||
| .segment-page-heatmap { | ||
| --hex-w: clamp(60px, 5.5vw, 85px); |
There was a problem hiding this comment.
add comments with explanation for css
| private subscriptions: Subscription[] = []; | ||
|
|
||
| segment = input.required<Segment>(); | ||
| temperature!: number; |
There was a problem hiding this comment.
abstract to be able to take in multiple values, that have format functions tied to them
| @@ -0,0 +1,29 @@ | |||
| <!-- Unified segment row: dark wrapper with light heatmap and overview panels --> | |||
There was a problem hiding this comment.
Going to make ticket to abstract to be a general wrapper that can have two related parts, then use that abstracted component here. Not supper necessary but will be nice to have and a lot easier to maintain.
| display: block; | ||
| } | ||
|
|
||
| /* ── Segment Row ── */ |
There was a problem hiding this comment.
add ai-gen ack, and more comments
| overflow: visible !important; /* Allow dropdown to overflow */ | ||
| } | ||
|
|
||
| /* ── Section header row ── */ |
There was a problem hiding this comment.
AI gen ack and more comments
| </mat-grid-list> | ||
|
|
||
| <!-- Section header with "Set ALL Maps" dropdown --> | ||
| <div class="section-header"> |
There was a problem hiding this comment.
abstract section header to it's own re-usable component, with left and right title inputs and select drop down options (allow for multiple select dropdowns, via a list of selector configs.)
Changes
Combines the segment summary stats and cell heatmap into unified segment rows on the BMS debug page. Each row has a dark controls zone (segment label, view selector, "See more" link) and a light content zone with the hex-tile heatmap and overview stats side by side.
Key architectural decisions:
SegmentRowComponent,SegmentHeatmapComponent,SegmentOverviewComponent, andHexTileComponent— each segment row is self-contained with its own view selectorCellReadingto a single-cell model (from paired cells) so each hex tile maps 1:1 to a cell reading. This simplifies the data flow and makes the heatmap config-driven viaBMS_CONFIGBMS_CONFIGtobms.config.tsto break a circular import betweenbms.utils.tsandtopic.utils.tsALPHA_THERM_CELL_MAP/BETA_THERM_CELL_MAP. The double-hex shape uses a V-notch clip-path polygon to visually distinguish temperature tiles from single-cell voltage/balancing tilesHeatMapService— aglobalView$BehaviorSubject drives the "Set ALL Maps" selector and the initial default for per-row selectors. Per-row selectors subscribe to their segment's view and stay in sync when the global selector changes. TheSelectDropdownComponentnow uses aneffect()to reactively trackdefaultValuechangesclamp()for hex tiles and row heights to support 70–150% browser zoomNotes
#525commits at the base are from the525-generalize-bms-cell-data-modelbranch which this depends onsetIntervalpolling to refresh MQTT values — this could be replaced with proper observable subscriptions in a follow-upTest Cases
Checklist
It can be helpful to check the
ChecksandFiles changedtabs.Please review the contributor guide and reach out to your Tech Lead if anything is unclear.
Please request reviewers and ping on slack only after you've gone through this whole checklist.
package-lock.jsonchanges (unless dependencies have changed)Closes #527