-
Notifications
You must be signed in to change notification settings - Fork 0
limelight-recorder #6
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
base: master
Are you sure you want to change the base?
Conversation
| import "./App.css"; | ||
| import LimelightTable from "./LimelightTable"; | ||
|
|
||
| function App() { |
Check failure
Code scanning / ESLint
Enforce naming conventions for everything across a codebase Error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider making this a variable: const App: FC = () => {}
|
|
||
| function App() { | ||
| const [message, setMessage] = useState("Loading..."); | ||
| const [robotOnline, setRobotOnline] = useState(false); |
Check failure
Code scanning / ESLint
Enforce naming conventions for everything across a codebase Error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isRobotOnline
|
|
||
| useEffect(() => { | ||
| fetch("http://localhost:5000/") | ||
| .then((res) => res.text()) |
Check failure
Code scanning / ESLint
Require any function or method that returns a Promise to be marked async Error
| fetch("http://localhost:5000/") | ||
| .then((res) => res.text()) | ||
| .then(setMessage) | ||
| .catch((e) => { |
Check failure
Code scanning / ESLint
Enforce typing arguments in Promise rejection callbacks as `unknown` Error
| const interval = setInterval(async () => { | ||
| try { | ||
| const res = await fetch("http://localhost:4590/"); | ||
| const text = await res.text(); | ||
| setMessage(text); | ||
| setRobotOnline(text.includes("Welcome")); | ||
| } catch { | ||
| setRobotOnline(false); | ||
| } | ||
| }, 2000); |
Check failure
Code scanning / ESLint
Disallow Promises in places not designed to handle them Error
|
|
||
| // Logging | ||
| this.ffmpegProcess.stderr?.on("data", (data) => { | ||
| console.log(data.toString()); |
Check warning
Code scanning / ESLint
Disallow member access on a value with type `any` Warning
|
|
||
| const app = express(); | ||
| const port = 5000; | ||
| app.use(cors()); |
Check warning
Code scanning / ESLint
Disallow calling a value with type `any` Warning
| import "./App.css"; | ||
| import LimelightTable from "./LimelightTable"; | ||
|
|
||
| function App() { |
Check warning
Code scanning / ESLint
Require explicit return and argument types on exported functions' and classes' public class methods Warning
| import './index.css' | ||
| import App from './App' | ||
|
|
||
| createRoot(document.getElementById('root')!).render( |
Check warning
Code scanning / ESLint
Disallow non-null assertions using the `!` postfix operator Warning
| import './index.css' | ||
| import App from './App' | ||
|
|
||
| createRoot(document.getElementById('root')!).render( |
Check warning
Code scanning / ESLint
Disallow non-null assertions using the `!` postfix operator Warning
YoniKiriaty
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice
| function startRecording() { | ||
| if (ffmpegProcessLeft || ffmpegProcessObject || ffmpegProcessRight) { | ||
| return; | ||
| } | ||
|
|
||
| let sessionDir: string; | ||
|
|
||
| try { | ||
| sessionDir = createSessionFolder(); | ||
| } catch (err) { | ||
| console.error(err); | ||
| return; | ||
| } | ||
|
|
||
| ffmpegProcessLeft = new RecordingProcess( | ||
| "left", | ||
| path.join(sessionDir, "left.mp4") | ||
| ); | ||
| ffmpegProcessLeft.startRecording(); | ||
|
|
||
| ffmpegProcessObject = new RecordingProcess( | ||
| "object", | ||
| path.join(sessionDir, "object.mp4") | ||
| ); | ||
| ffmpegProcessObject.startRecording(); | ||
|
|
||
| ffmpegProcessRight = new RecordingProcess( | ||
| "right", | ||
| path.join(sessionDir, "right.mp4") | ||
| ); | ||
| ffmpegProcessRight.startRecording(); | ||
|
|
||
| console.log(`Recording started in ${sessionDir}`); | ||
| } | ||
|
|
||
| function stopRecording() { | ||
| // Stop left camera | ||
| if (ffmpegProcessLeft) { | ||
| ffmpegProcessLeft.stopRecording(); | ||
| ffmpegProcessLeft = null; | ||
| console.log("Stopped recording: left"); | ||
| } | ||
|
|
||
| // Stop object camera | ||
| if (ffmpegProcessObject) { | ||
| ffmpegProcessObject.stopRecording(); | ||
| ffmpegProcessObject = null; | ||
| console.log("Stopped recording: object"); | ||
| } | ||
|
|
||
| // Stop right camera | ||
| if (ffmpegProcessRight) { | ||
| ffmpegProcessRight.stopRecording(); | ||
| ffmpegProcessRight = null; | ||
| console.log("Stopped recording: right"); | ||
| } | ||
| } | ||
|
|
||
| const oneSecond = 1000; | ||
| async function pingRobot(robotIp: string) { | ||
| const result = await ping.promise.probe(robotIp, { timeout: 10 }); | ||
| return result; | ||
| } | ||
| // --- PING CAMERAS --- | ||
| setInterval(() => { | ||
| async function pingCameras () { | ||
| const robotIp = "10.45.90.2"; | ||
| const isUp = await pingRobot(robotIp).then((res) => res); | ||
|
|
||
| if (isUp.alive) { | ||
| console.log(`Robot at ${robotIp} is online.`); | ||
| startRecording(); | ||
| } | ||
|
|
||
| if (!isUp.alive) { | ||
| console.log(`Robot at ${robotIp} is offline.`); | ||
| stopRecording(); | ||
| } | ||
| } | ||
| pingCameras().catch(() => { | ||
| console.error("Couldnt ping cameras"); | ||
| }) | ||
| }, oneSecond); No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider making this a function in another file and importing and using it, as using it in the main file is kind of a lot.
| function createSessionFolder(): string { | ||
| if (!fs.existsSync(USB_ROOT)) { | ||
| throw new Error("USB drive not connected"); | ||
| } | ||
|
|
||
| const timestamp = new Date().toISOString().replace(/[:.]/g, "-"); | ||
| const sessionDir = path.join(USB_ROOT, `recording-${timestamp}`); | ||
|
|
||
| fs.mkdirSync(sessionDir, { recursive: true }); | ||
| return sessionDir; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider moving this too
| try { | ||
| sessionDir = createSessionFolder(); | ||
| } catch (err) { | ||
| console.error(err); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider moving this try catch into the createSessionFolder function, and return undefined in the createSessionFolder, then return if sessionDir is undefined
| ffmpegProcessLeft = new RecordingProcess( | ||
| "left", | ||
| path.join(sessionDir, "left.mp4") | ||
| ); | ||
| ffmpegProcessLeft.startRecording(); | ||
|
|
||
| ffmpegProcessObject = new RecordingProcess( | ||
| "object", | ||
| path.join(sessionDir, "object.mp4") | ||
| ); | ||
| ffmpegProcessObject.startRecording(); | ||
|
|
||
| ffmpegProcessRight = new RecordingProcess( | ||
| "right", | ||
| path.join(sessionDir, "right.mp4") | ||
| ); | ||
| ffmpegProcessRight.startRecording(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this code is repetitive. consider using an object to descripe the ffmpegProcesss. Initializing it as {left: undefined, object: undefined, right: undefined}, and then iterating over the object to initialize these.
| if (ffmpegProcessLeft) { | ||
| ffmpegProcessLeft.stopRecording(); | ||
| ffmpegProcessLeft = null; | ||
| console.log("Stopped recording: left"); | ||
| } | ||
|
|
||
| // Stop object camera | ||
| if (ffmpegProcessObject) { | ||
| ffmpegProcessObject.stopRecording(); | ||
| ffmpegProcessObject = null; | ||
| console.log("Stopped recording: object"); | ||
| } | ||
|
|
||
| // Stop right camera | ||
| if (ffmpegProcessRight) { | ||
| ffmpegProcessRight.stopRecording(); | ||
| ffmpegProcessRight = null; | ||
| console.log("Stopped recording: right"); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as iterating here
| const zero = 0; | ||
| const one = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const zero = 0; | |
| const one = 1; | |
| const leftCameraIndex = 0; | |
| const objectCameraIndex = 1; |
| declare module "react" { | ||
| interface InputHTMLAttributes<T> extends HTMLAttributes<T> { | ||
| webkitdirectory?: boolean; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't webkitdirectory already in HTMLAttributes?
| await fetch(`http://localhost:5000/record/start/${camera}`, { | ||
| method: "POST", | ||
| }); | ||
| } else if (!robotOnline) { | ||
| await fetch(`http://localhost:5000/record/stop/${camera}`, { | ||
| method: "POST", | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make sure these ports are the correct ones
| const zero = 0; | ||
| const one = 1; | ||
|
|
||
| async function doThingy( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider naming this something indicative
| } | ||
|
|
||
| const LimelightTable: React.FC<LimelightTableProps> = ({ robotOnline }) => { | ||
| const cameraStatuses = [false, false, false]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider having this as an object: {left: false, object: false, right: false}
No description provided.