-
Notifications
You must be signed in to change notification settings - Fork 0
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
Use Bacon for packet reads #10
Conversation
fchorney
commented
Apr 5, 2024
- Made SMXStage class to hook into all the bacon stuff
- Modified everything else to mostly work with it
- Probably did some dumb stuff
- Made SMXStage class to hook into all the bacon stuff - Modified everything else to mostly work with it - Probably did some dumb stuff
@@ -56,24 +17,36 @@ function hasValue<T>(ev: Bacon.Event<T>): ev is Bacon.Value<T> { | |||
return ev.hasValue; | |||
} | |||
|
|||
type Packet = { type: "host_cmd_finished" } | { type: "data"; payload: Uint8Array }; | |||
export type Packet = { type: "host_cmd_finished"; payload: [] } | { type: "data"; payload: Uint8Array }; |
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.
I did this because I was sick of checking of the type was data every time I wanted to grab the payload.
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.
Also, I wonder if we can use the host_cmd_finished
packet when we make our output pipes to know when we can send the next packet down the line.
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.
Yes we definitely need to throttle our writes based on the readystate communicated back here. I think I know how to do that...
Also, if you don't want to be stuck always checking for a payload then it sounds like we need another downstream filter on the other event streams that filters out the cmd finished responses.
await this.updateConfig(); | ||
|
||
// Request some initial test data | ||
await this.updateTestData(); |
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.
So, for some reason this doesn't actually get a response when it's run here and I can't figure out why (sending requests too fast down the line maybe?) Works fine when you request it from the debug buttons on the site.
ui/DebugCommands.tsx
Outdated
return; | ||
} | ||
// TODO: This actually works, but I don't know how to stop it from complaining | ||
await stage[cmd as keyof SMXStage](); |
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 is some crazy jibberish I found online, not sure what the better method would be here with regards to changing how DEBUG_COMMANDS
is defined as well.
uiState.set(stages$, (stages) => { | ||
return { | ||
...stages, | ||
[stage.info?.player || 0]: stage, // TODO: Is there a better way to handle this? |
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.
maybe some sort of error handling if stage.info still isn't populated by now?
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.
I don't think .then
even actually gets us in the right spot. We somehow want to resolve this promise when we get responses for info and config I think. Not too sure how to do that yet
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.
Yup, resolving when the response gets ack'd is the way to go. Will look into that soon
if (data) { | ||
setTestData(data); | ||
} | ||
await d.updateTestData(SensorTestMode.CalibratedValues); |
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.
At this point it would be nice to have a dropdown in the debug stuff to select what test mode we want to use.
const inputState = useInputState(device); | ||
const stage = useAtomValue(stageAtom); | ||
const testData = useTestData(stage); | ||
// const inputState = useInputState(stage); |
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.
We can probably figure out how to enable this again I just didn't have time tonight