Feat/analytics api and admin charts #1
Conversation
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
- Added firebase-admin version 13.3.0 to package.json and package-lock.json - Updated various dependencies in package-lock.json for improved stability and performance
- Add a new GET method in route.ts to fetch sensor readings from Firestore - Return the latest 500 readings in JSON format, handling errors appropriately - Ensure Firestore query is optimized with index usage
- Create a new file for Firebase admin setup - Configure Firebase admin with environment variables for secure access - Initialize Firestore database for backend interactions
- Add a new receiver.ino file for handling LoRa data reception - Implement AES decryption for incoming payloads - Set up Wi-Fi, Firebase, and NTP client for data synchronization - Parse received LoRa data and upload sensor readings to Firebase - Update OLED display with temperature, humidity, and heat index values
- Replace the previous AES decryption implementation with a more efficient version - Update the parsing logic for decrypted data from LoRa - Dynamically set the STATION_ID based on the device's MAC address - Clean up unused functions and improve code organization for better readability
- Implemented REAL TIME functionality to update the Realtime Database with temperature, humidity, heat index, and timestamp data
…irebase integration - Removed AES decryption for incoming LoRa data, simplifying the payload handling process. Will add later - Updated parsing logic to directly process received payloads - Enhanced Firebase Firestore document creation with clearer collection management - Cleaned up unused code and improved overall readability
…updates - Introduced RECEIVER_ID based on the device's MAC address for better identification - Updated Firebase Firestore and Realtime Database interactions to include receiver ID - Cleared sensitive information from the configuration for security purposes
SEAN: - Implemented a new sender.ino file to handle LoRa data transmission - Integrated DHT sensor for temperature and humidity readings - Added relay control for power management based on sensor data - Established LoRa communication setup with configurable frequency and sync word
…n LoRa sender - Added WiFi library to obtain MAC address for unique sensor identification - Initialized SENSOR_ID based on the device's MAC address - Enhanced error handling for DHT sensor readings - Cleaned up code formatting and improved readability
…and sender - Removed unnecessary emoji from comments for clarity - Enhanced formatting and spacing in sender.ino for better readability - Streamlined comment structure in receiver.ino to maintain consistency
|
This is a benchmark review for experiment This pull request was cloned from Experiment configurationreview_config:
# User configuration for the review
# - benchmark - use the user config from the benchmark reviews
# - <value> - use the value directly
user_review_config:
enable_ai_review: true
enable_rule_comments: false
enable_complexity_comments: benchmark
enable_security_comments: benchmark
enable_tests_comments: benchmark
enable_comment_suggestions: benchmark
enable_pull_request_summary: benchmark
enable_review_guide: benchmark
enable_approvals: false
base_branches: [base-sha.*]
ai_review_config:
# The model responses to use for the experiment
# - benchmark - use the model responses from the benchmark reviews
# - llm - call the language model to generate responses
model_responses:
comments_model: benchmark
comment_validation_model: benchmark
comment_suggestion_model: benchmark
complexity_model: benchmark
security_model: benchmark
tests_model: benchmark
pull_request_summary_model: benchmark
review_guide_model: benchmark
overall_comments_model: benchmark
# The pull request dataset to run the experiment on
pull_request_dataset:
# CodeRabbit
- https://github.com/neerajkumar161/node-coveralls-integration/pull/5
- https://github.com/gunner95/vertx-rest/pull/1
- https://github.com/Altinn/altinn-access-management-frontend/pull/1427
- https://github.com/theMr17/github-notifier/pull/14
- https://github.com/bearycool11/AI_memory_Loops/pull/142
# Greptile
- https://github.com/gumloop/guMCP/pull/119
- https://github.com/autoblocksai/python-sdk/pull/335
- https://github.com/grepdemos/ImageSharp/pull/6
- https://github.com/grepdemos/server/pull/61
- https://github.com/websentry-ai/pipelines/pull/25
# Graphite
- https://github.com/KittyCAD/modeling-app/pull/6648
- https://github.com/KittyCAD/modeling-app/pull/6628
- https://github.com/Varedis-Org/AI-Test-Repo/pull/2
- https://github.com/deeep-network/bedrock/pull/198
- https://github.com/Metta-AI/metta/pull/277
# Copilot
- https://github.com/hmcts/rpx-xui-webapp/pull/4438
- https://github.com/ganchdev/quez/pull/104
- https://github.com/xbcsmith/ymlfxr/pull/13
- https://github.com/tinapayy/B-1N1T/pull/36
- https://github.com/coder/devcontainer-features/pull/6
# Questions to ask to label the review comments
review_comment_labels: []
# - label: correct
# question: Is this comment correct?
# Benchmark reviews generated by running
# python -m scripts.experiment benchmark <experiment_name>
benchmark_reviews: []
|
Reviewer's GuideThis pull request introduces Arduino firmware for LoRa-based sensor data collection and a Next.js API endpoint to retrieve this data. The receiver firmware uploads sensor readings (temperature, humidity, heat index) received via LoRa to Firestore and Firebase RTDB. The new API route File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Hellebore
commented
May 2, 2025
There was a problem hiding this comment.
Hey @Hellebore - I've reviewed your changes - here's some feedback:
- Consider moving sensitive credentials like WiFi passwords and Firebase keys out of the
receiver.inosource code, perhaps using a configuration file or environment variables during flashing. - Firebase Admin credentials should remain server-side only; avoid using the
NEXT_PUBLIC_prefix for these environment variables infirebase-admin.ts. - The analytics API route fetches a fixed number of 500 readings; consider adding parameters for pagination or filtering if the frontend might need more specific data subsets.
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟡 Security: 1 issue found
- 🟢 Testing: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| timeClient.update(); | ||
| } | ||
|
|
||
| void parsePlainPayload(String payload) { |
There was a problem hiding this comment.
suggestion (bug_risk): Improve string parsing reliability in parsePlainPayload.
Use a dedicated split method or explicitly validate each token before converting to float to handle malformed or extra delimiters.
Suggested implementation:
#include <stdlib.h>
// Helper function to validate if a string is a valid float number.
bool isValidFloat(String s) {
// Convert String to a C string then use strtof to check conversion.
char buf[s.length() + 1];
s.toCharArray(buf, s.length() + 1);
char *end;
strtof(buf, &end);
return (*end == '\0');
}
// Helper function to split a string by a given delimiter.
// Returns the number of tokens found.
int splitString(String str, char delimiter, String tokens[], int maxTokens) {
int tokenCount = 0;
int pos = 0;
int index = 0;
while ((pos = str.indexOf(delimiter)) != -1 && tokenCount < maxTokens) {
tokens[tokenCount++] = str.substring(0, pos);
str = str.substring(pos + 1);
}
// Add the last token if any and if there's room.
if (tokenCount < maxTokens) {
tokens[tokenCount++] = str;
}
return tokenCount;
}
void parsePlainPayload(String payload) {
String data[3];
int count = splitString(payload, ';', data, 3);
// Validate each token before converting to float.
if (count >= 1 && isValidFloat(data[0])) {
temperature = data[0].toFloat();
} else {
temperature = 0.0; // Default value or error handling
}
if (count >= 2 && isValidFloat(data[1])) {
humidity = data[1].toFloat();
} else {
humidity = 0.0; // Default value or error handling
}
Ensure to adjust any references to variables (like "temperature" and "humidity") if needed, and include necessary error handling or logging mechanisms according to your project's standards.
| unsigned long relayInterval = 10UL * 1000UL; // 60 seconds in milliseconds | ||
| bool useInitialPowerbank = true; | ||
|
|
||
| void startDHT() |
There was a problem hiding this comment.
suggestion: Clarify the purpose of startDHT.
Since it only checks sensor readings and logs failures rather than initializing, consider renaming it to verifyDHT for clarity.
Suggested implementation:
void verifyDHT()
Serial.println("DHT sensor check successful!");
If there are any calls to startDHT() elsewhere in the code, please update them to call verifyDHT() instead.
Comment on lines
+33
to
+42
| unsigned long relayInterval = 10UL * 1000UL; // 60 seconds in milliseconds | ||
| bool useInitialPowerbank = true; | ||
|
|
||
| void startDHT() | ||
| { | ||
| if (isnan(humidity) || isnan(temperature)) | ||
| { | ||
| Serial.println("Failed to read from DHT sensor!"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
question: Clarify relayInterval values.
Please clarify why relayInterval starts at 10 s but then switches to 60 s after the first relay by adding an explanatory comment.
Comment on lines
+5
to
+9
| credential: cert({ | ||
| projectId: process.env.NEXT_PUBLIC_FIREBASE_PROJECT_ID, | ||
| clientEmail: process.env.NEXT_PUBLIC_FIREBASE_CLIENT_EMAIL, | ||
| privateKey: process.env.NEXT_PUBLIC_FIREBASE_PRIVATE_KEY?.replace(/\\n/g, "\n"), | ||
| }), |
There was a problem hiding this comment.
🚨 suggestion (security): Use non-public env vars for firebase admin credentials.
NEXT_PUBLIC_ vars are exposed to the client. Use private (non-client-accessible) env vars for firebase-admin credentials to keep them secure.
Suggested change
| credential: cert({ | |
| projectId: process.env.NEXT_PUBLIC_FIREBASE_PROJECT_ID, | |
| clientEmail: process.env.NEXT_PUBLIC_FIREBASE_CLIENT_EMAIL, | |
| privateKey: process.env.NEXT_PUBLIC_FIREBASE_PRIVATE_KEY?.replace(/\\n/g, "\n"), | |
| }), | |
| credential: cert({ | |
| projectId: process.env.FIREBASE_PROJECT_ID, | |
| clientEmail: process.env.FIREBASE_CLIENT_EMAIL, | |
| privateKey: process.env.FIREBASE_PRIVATE_KEY?.replace(/\\n/g, "\n"), | |
| }), |
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
Implement analytics API and hardware sensor communication system for environmental monitoring
New Features:
Enhancements:
Deployment:
Chores: