Skip to content

Telemetry via serial port #97

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Telemetry via serial port #97

wants to merge 3 commits into from

Conversation

MCLiii
Copy link
Member

@MCLiii MCLiii commented May 6, 2024

No description provided.

@MCLiii MCLiii force-pushed the serial-telemetry branch from da69224 to 64819bd Compare May 16, 2024 14:51
@MCLiii MCLiii force-pushed the serial-telemetry branch from 64819bd to b9426c5 Compare May 17, 2024 17:37
@sbasu107 sbasu107 requested a review from Copilot June 10, 2025 04:48
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds end-to-end serial telemetry support by exposing available ports in the frontend, wiring up selection in the dashboard, and implementing serial read logic in the backend.

  • Introduce a new SerialSelector component and embed it in the Dashboard UI
  • Add /serial-info and /connect-device endpoints and a serial_read loop in backend comms
  • Update dependencies (pyserial) and adjust startup logging in main.py

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
Frontend/src/Components/SerialSelector/SerialSelector.js New component to list and select serial devices and baud rates
Frontend/src/Components/Dashboard/Dashboard.js Import and layout <SerialSelector/> in the dashboard grid
Frontend/src/Components/Communication/Communication.js Add serial status label; removed solar car connection label
Backend/setup.py Add pyserial to install_requires
Backend/main.py Set uvicorn log level to critical
Backend/core/core_api.py Implement /serial-info GET and /connect-device POST routes
Backend/core/comms.py Add serial_read thread, update packet parsing for serial input
Backend/.gitignore Ignore files from file sync downloads
Comments suppressed due to low confidence (4)

Backend/core/comms.py:146

  • The time module is used here but not imported; add import time at the top of the file.
if(time.time() - latest_tstamp > 5):

Frontend/src/Components/Communication/Communication.js:55

  • The Solar Car Connection label was removed—if displaying that status is intended, re-add this CommsLabel.
<CommsLabel
                    boolean={props.data?.solar_car_connection[0]}
                    label='Solar Car Connection'
                />

Backend/core/comms.py:178

  • The traceback module is used without import; add import traceback at the top of the file.
print(traceback.format_exc())

Backend/main.py:14

  • [nitpick] Using log_level='critical' silences most runtime logs; consider a less restrictive default or make it configurable.
uvicorn.run(app='main:app', host="0.0.0.0", port=config.HOST_PORT, log_level='critical')

}
})
.then((body) => {
setSelectedDevice(body['connected_device']['device']); // Set default device
Copy link
Preview

Copilot AI Jun 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refreshing the selected device on every interval will override the user's choice. Consider only setting the default on initial mount or when selectedDevice is empty.

Suggested change
setSelectedDevice(body['connected_device']['device']); // Set default device
if (!selectedDevice) { // Only set default device if no device is selected
setSelectedDevice(body['connected_device']['device']);
}

Copilot uses AI. Check for mistakes.

width={'30%'}
padding={2}
value={selectedBaud}
onChange={e => setSelectedBaud(e.target.value)}
Copy link
Preview

Copilot AI Jun 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

e.target.value is a string but selectedBaud is numeric—wrap with parseInt(e.target.value, 10) to ensure the state stays a number.

Suggested change
onChange={e => setSelectedBaud(e.target.value)}
onChange={e => setSelectedBaud(parseInt(e.target.value, 10))}

Copilot uses AI. Check for mistakes.

header = b'<bsr>'
footer = b'</bsr>'
header = b"<bsr>"
footer = b"<bsr"
Copy link
Preview

Copilot AI Jun 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Footer tag is missing the closing slash—should be b'</bsr>' to match the regex and proper packet parsing.

Suggested change
footer = b"<bsr"
footer = b"</bsr>"

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant