Skip to content

Commit abc0f56

Browse files
committed
refactor: simplify battery history implementation for better maintainability
1 parent 90ddc9a commit abc0f56

File tree

4 files changed

+43
-389
lines changed

4 files changed

+43
-389
lines changed

CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2222
- Increased record limit for multi-day views to provide better data coverage
2323
- Enhanced logging for better troubleshooting of time filter issues
2424

25+
### Improved
26+
- Simplified battery history implementation:
27+
- Streamlined data fetching and processing in useBatteryHistory hook
28+
- Reduced complexity in batteryHistoryService by using a single reliable endpoint
29+
- Simplified chart configuration in Dashboard component
30+
- Removed excessive logging and error handling
31+
- Improved code readability and maintainability
32+
2533
## [1.9.0] - 2025-03-01
2634

2735
### Changed

client/src/hooks/useBatteryHistory.js

Lines changed: 19 additions & 151 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@ import { fetchBatteryHistory, recordBatteryCharge } from '../services/batteryHis
44
/**
55
* Custom hook for fetching and managing battery history data
66
* @param {number|null} upsId - The ID of the UPS system to fetch history for
7-
* @param {number} timeFilter - Number of days to fetch history for (default: 7)
7+
* @param {number} timeFilter - Number of days to fetch history for (default: 3)
88
* @returns {Object} - Battery history data and loading state
99
*/
10-
export const useBatteryHistory = (upsId, timeFilter = 7) => {
10+
export const useBatteryHistory = (upsId, timeFilter = 3) => {
1111
const [batteryHistory, setBatteryHistory] = useState({
1212
labels: [],
1313
datasets: [],
@@ -23,170 +23,41 @@ export const useBatteryHistory = (upsId, timeFilter = 7) => {
2323

2424
// Function to fetch and process battery history data
2525
const fetchAndProcessData = async (days = currentTimeFilter) => {
26+
if (!upsId) return;
27+
2628
try {
2729
setLoading(true);
2830
setError(null);
2931

30-
console.log(`Fetching battery history for UPS ${upsId} with time filter: ${days} days`);
31-
32-
// Use our service to fetch the data
32+
// Fetch the data
3333
const data = await fetchBatteryHistory(upsId, days);
3434

35-
console.log(`Received ${data?.length || 0} data points for the last ${days} days`);
36-
3735
if (!data || data.length === 0) {
38-
console.log('No battery history data available');
3936
setBatteryHistory({
4037
labels: [],
4138
datasets: [],
4239
});
4340
return;
4441
}
4542

46-
console.log('Processing battery history data:', data);
47-
48-
// Process the data for the chart
49-
// The API returns an array of objects with charge_percent and timestamp
50-
console.log('Raw data from API:', data);
51-
5243
// Sort the data by timestamp
5344
const sortedData = [...data].sort((a, b) => new Date(a.timestamp) - new Date(b.timestamp));
5445

55-
console.log(`Sorted data: ${sortedData.length} records`);
56-
57-
// Even though the server should filter the data, we'll also filter it on the client side
58-
// to ensure we only show data for the requested time period
59-
const now = new Date();
60-
const oldestAllowedDate = new Date(now);
61-
oldestAllowedDate.setDate(oldestAllowedDate.getDate() - days);
62-
console.log(`Client-side filtering: Only including records after ${oldestAllowedDate.toLocaleString()}`);
63-
64-
// Filter the data to only include records within the requested time period
65-
const filteredData = sortedData.filter(item => {
66-
const timestamp = new Date(item.timestamp);
67-
return timestamp >= oldestAllowedDate;
46+
// Format timestamps for display
47+
const formattedLabels = sortedData.map(item => {
48+
const date = new Date(item.timestamp);
49+
return date.toLocaleString([], {
50+
month: 'short',
51+
day: 'numeric',
52+
hour: '2-digit',
53+
minute: '2-digit'
54+
});
6855
});
6956

70-
console.log(`After client-side filtering: ${filteredData.length} records (was ${sortedData.length})`);
71-
72-
// For multi-day views, ensure we get a good representation of the data across the full time range
73-
let finalData = filteredData;
74-
75-
if (days > 1) {
76-
console.log(`Ensuring good data representation for ${days}-day view`);
77-
78-
// Calculate the time range we want to cover (from now to N days ago)
79-
const now = new Date();
80-
const oldestAllowedDate = new Date(now);
81-
oldestAllowedDate.setDate(oldestAllowedDate.getDate() - days);
82-
83-
// If we don't have data going back the full requested time range,
84-
// we'll work with what we have and distribute it evenly
85-
86-
// Calculate how many data points we want per day (aim for about 12)
87-
const totalDesiredPoints = days * 12;
88-
89-
if (filteredData.length > totalDesiredPoints) {
90-
// If we have more points than desired, sample them
91-
const samplingInterval = Math.max(1, Math.floor(filteredData.length / totalDesiredPoints));
92-
93-
console.log(`Sampling interval: ${samplingInterval} (keeping 1 out of every ${samplingInterval} points)`);
94-
95-
// Sample the data at regular intervals
96-
finalData = filteredData.filter((_, index) => index % samplingInterval === 0);
97-
98-
// Always include the first and last points to ensure we cover the full time range
99-
if (finalData[0] !== filteredData[0]) {
100-
finalData.unshift(filteredData[0]);
101-
}
102-
if (finalData[finalData.length - 1] !== filteredData[filteredData.length - 1]) {
103-
finalData.push(filteredData[filteredData.length - 1]);
104-
}
105-
106-
console.log(`After sampling: ${finalData.length} records (was ${filteredData.length})`);
107-
} else {
108-
// If we have fewer points than desired, use all of them
109-
console.log(`Using all ${filteredData.length} records (fewer than desired ${totalDesiredPoints})`);
110-
111-
// If we have very few points, we might want to interpolate additional points
112-
// to get a smoother chart, but for now we'll just use what we have
113-
}
114-
}
115-
116-
// Check if the data has the expected properties
117-
let timestamps, charges;
118-
119-
if (finalData.length > 0 && !finalData[0].charge_percent) {
120-
console.warn('Data format issue: charge_percent not found in data');
121-
console.log('Data sample:', finalData[0]);
122-
123-
// Try to detect the correct property names
124-
const sampleItem = finalData[0];
125-
const chargeKey = Object.keys(sampleItem).find(key =>
126-
key.includes('charge') || key.includes('battery') ||
127-
(typeof sampleItem[key] === 'number' && sampleItem[key] >= 0 && sampleItem[key] <= 100)
128-
);
129-
130-
const timeKey = Object.keys(sampleItem).find(key =>
131-
key.includes('time') || key.includes('date') ||
132-
(typeof sampleItem[key] === 'string' && sampleItem[key].includes('T'))
133-
);
134-
135-
if (chargeKey && timeKey) {
136-
console.log(`Using detected keys: ${chargeKey} for charge and ${timeKey} for timestamp`);
137-
timestamps = finalData.map(item => item[timeKey]);
138-
charges = finalData.map(item => item[chargeKey]);
139-
} else {
140-
console.error('Could not detect appropriate data keys');
141-
setBatteryHistory({
142-
labels: [],
143-
datasets: [],
144-
});
145-
return;
146-
}
147-
} else {
148-
timestamps = finalData.map(item => item.timestamp);
149-
charges = finalData.map(item => item.charge_percent);
150-
}
151-
152-
// Store the original Date objects for logging
153-
const dateObjects = timestamps.map(ts => new Date(ts));
154-
155-
// Format timestamps for better display
156-
const formattedLabels = dateObjects.map(date => {
157-
return date.toLocaleString([], {
158-
month: 'short',
159-
day: 'numeric',
160-
hour: '2-digit',
161-
minute: '2-digit'
162-
});
163-
});
164-
165-
// Log the date range of the data
166-
if (dateObjects.length > 0) {
167-
const oldestDate = dateObjects[0];
168-
const newestDate = dateObjects[dateObjects.length - 1];
169-
console.log(`Data date range: ${oldestDate.toLocaleString()} to ${newestDate.toLocaleString()}`);
170-
171-
// Calculate how many hours of data we have
172-
const hoursDiff = (newestDate - oldestDate) / (1000 * 60 * 60);
173-
console.log(`Data spans approximately ${hoursDiff.toFixed(1)} hours (${(hoursDiff / 24).toFixed(1)} days)`);
174-
175-
// Check if the data matches the requested time filter
176-
const now = new Date();
177-
const requestedOldestDate = new Date(now);
178-
requestedOldestDate.setDate(requestedOldestDate.getDate() - days);
179-
console.log(`Requested date range: ${requestedOldestDate.toLocaleString()} to ${now.toLocaleString()}`);
180-
181-
// Calculate the difference between the oldest date in the data and the requested oldest date
182-
const diffHours = (oldestDate - requestedOldestDate) / (1000 * 60 * 60);
183-
console.log(`Difference between requested and actual oldest date: ${diffHours.toFixed(1)} hours`);
184-
185-
if (diffHours > 24) {
186-
console.warn(`Warning: Data does not go back as far as requested. Missing approximately ${diffHours.toFixed(1)} hours of data.`);
187-
}
188-
}
57+
// Extract charge percentages
58+
const charges = sortedData.map(item => item.charge_percent);
18959

60+
// Create chart data
19061
setBatteryHistory({
19162
labels: formattedLabels,
19263
datasets: [
@@ -202,7 +73,7 @@ export const useBatteryHistory = (upsId, timeFilter = 7) => {
20273
});
20374
} catch (err) {
20475
console.error('Error fetching battery history:', err);
205-
setError(err.message || 'Failed to fetch battery history');
76+
setError('Failed to fetch battery history');
20677

20778
// Set empty chart data on error
20879
setBatteryHistory({
@@ -221,7 +92,6 @@ export const useBatteryHistory = (upsId, timeFilter = 7) => {
22192
try {
22293
setLoading(true);
22394
await recordBatteryCharge(upsId, currentCharge);
224-
console.log(`Manually recorded battery charge: ${currentCharge}%`);
22595

22696
// Refresh the data after recording
22797
await fetchAndProcessData();
@@ -234,7 +104,6 @@ export const useBatteryHistory = (upsId, timeFilter = 7) => {
234104
};
235105

236106
useEffect(() => {
237-
if (!upsId) return;
238107
fetchAndProcessData(currentTimeFilter);
239108
}, [upsId, currentTimeFilter]);
240109

@@ -244,7 +113,6 @@ export const useBatteryHistory = (upsId, timeFilter = 7) => {
244113
error,
245114
recordCurrentCharge,
246115
refreshData: fetchAndProcessData,
247-
currentTimeFilter,
248-
setTimeFilter: setCurrentTimeFilter
116+
currentTimeFilter
249117
};
250118
};

0 commit comments

Comments
 (0)