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

Increase readability of ctrl.c #214

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

Increase readability of ctrl.c #214

wants to merge 3 commits into from

Conversation

juhum1
Copy link

@juhum1 juhum1 commented Nov 10, 2024

added encoder enum, ternary operators, macros (Jacob)

@jacobkoziej jacobkoziej self-requested a review November 11, 2024 22:37
@jacobkoziej jacobkoziej assigned jacobkoziej and juhum1 and unassigned jacobkoziej Nov 11, 2024
Copy link
Member

@jacobkoziej jacobkoziej left a comment

Choose a reason for hiding this comment

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

Really good changes, my only additional request is a more descriptive PR name.

Comment on lines +87 to +90
pulse_cnt[ENCODER_N] += ((CHANNEL == ENCODER_CHANNEL_A) \
^ (chan_a ^ chan_b)) \
? 1 \
: -1; \
Copy link
Member

Choose a reason for hiding this comment

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

Let's disable clang formatting for the macro definitions so that we don't have strange formatting like this.

Comment on lines 179 to 180
const uint16_t cur_pulse_cnt[ENCODER_COUNT]
= {pulse_cnt[ENCODER_LEFT], pulse_cnt[ENCODER_RIGHT]};
Copy link
Member

Choose a reason for hiding this comment

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

Let's use [INDEX] = value formatting for this array.

@juhum1 juhum1 requested a review from jacobkoziej November 15, 2024 04:51
@juhum1 juhum1 changed the title Update ctrl.c Increase readability of ctrl.c Nov 15, 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.

2 participants