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

[SOFT 999] FW 103 HW #504

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

[SOFT 999] FW 103 HW #504

wants to merge 7 commits into from

Conversation

m6keller
Copy link

@m6keller m6keller commented Mar 2, 2022

FW 103 HW

@m6keller m6keller changed the title Soft 999 fw 103 hw [SOFT 999] FW 103 HW Mar 2, 2022
Copy link
Contributor

@mitchellostler mitchellostler left a comment

Choose a reason for hiding this comment

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

Looks good, but there are a few issues that will prevent the program from compiling. You should run make run PROJECT=fw_103_hw PLATFORM=x86

projects/fw_103_hw/src/main.c Outdated Show resolved Hide resolved
projects/fw_103_hw/src/main.c Outdated Show resolved Hide resolved
.priority = INTERRUPT_PRIORITY_HIGH,
};

adc_set_channel_pin(button_addr, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

The adc should be set for the potentiometer pin, instead of the button

projects/fw_103_hw/src/main.c Outdated Show resolved Hide resolved
Copy link
Contributor

@mitchellostler mitchellostler left a comment

Choose a reason for hiding this comment

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

FW 103 looks good, just a couple of fixes and you should be good to go. Were you able to get it to run?

Also a note: You should really put your name in your branch name, since there are other people using the same ticket number for homework



// GPIO address that we will allow us to read button data
const GpioAddress potentiometer_addr_A6 = { // could be passed as context
Copy link
Contributor

Choose a reason for hiding this comment

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

Good programming practice is to not to use global variables (ie any variable that is non-static declared outside a function). The const is good since they won't be changed, but declare them static as well const static

adc_init(ADC_MODE_SINGLE);

// GPIO Address where button is pressed
GpioAddress potentiometer_addr_B2 = {
Copy link
Contributor

Choose a reason for hiding this comment

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

This wouldn't be a potentiometer address, this would be a button address, so it would be good to label it as such

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.

2 participants