Skip to content
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

Added simulation project #256

Closed
wants to merge 3 commits into from

Conversation

cnscwong
Copy link
Contributor

No description provided.

@Bafran Bafran requested a review from JarvisWeng February 3, 2024 21:46
Copy link
Collaborator

@JarvisWeng JarvisWeng left a comment

Choose a reason for hiding this comment

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

Overall, pretty good job! Just a couple nits but otherwise its basically what I wanted

#include <arpa/inet.h>
#include <errno.h>
#include <fcntl.h>
#include <linux/can.h>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't need CAN

I2C_SET_READING,
SPI_SET_RX,
UART
} Operations;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe just add a NUM_OF_OPERATIONS at the end

input = operation;
switch (input) {
case GPIO_SET:
LOG_DEBUG("GPIO_SET\n");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hm could you just put each operation in a function? It just makes it easier to read

close(newsockfd);
close(socketfd);
}
#else
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is no if. This should only ever run on x86

}

#ifdef x86
int main(int argc, char *argv[]) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should be getting the port from the args, but this is fine for now

#define PORT 2520

int main() {
int socketfd;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also consider putting this socket logic into a function so other users don't need to copy this

@Bafran Bafran closed this Mar 23, 2024
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.

3 participants