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

SPI example has non-sensical inclusion of math expression that's actually longer than the number it represents #80

Closed
rwaldron opened this Issue Jul 15, 2016 · 5 comments

Comments

Projects
None yet
6 participants
@rwaldron
Copy link
Contributor

rwaldron commented Jul 15, 2016

var spi = new port.SPI({
  clockSpeed: 4*1000*1000, // 4MHz
  cpol: 1, // polarity
  cpha: 0, // clock phase
});

Is there a reason for assigning clockSpeed the value of an expression to be evaluated, versus just the number?

var spi = new port.SPI({
  clockSpeed: 4000000, // 4MHz
  cpol: 1, // polarity
  cpha: 0, // clock phase
});

Also, we should deprecate (with aliasing) clockSpeed, cpol and cphase in favor of parameters that are named something less hostile.

Recommending:

Old Name New Name
clockSpeed frequency
cpol polarity
cpha phase
@jandrieu

This comment has been minimized.

Copy link

jandrieu commented Sep 7, 2016

Representing large numbers like 4,000,000 as 4*1000*1000 instead of 4000000 makes them easier to read, debug, and maintain.

I'd keep the clockspeed as it is, but I agree more readable property names for polarity and phase would be an improvement.

@tcr

This comment has been minimized.

Copy link
Member

tcr commented Sep 7, 2016

If we were using Java/Python/Rust, representing it as 4_000_000 would be ideal. ;) Since this is for docs, it probably if we show the audience clock speed can just be represented as 4000000. (Persons writing their own codebases can use the expanded *1000 forms. Different audience.)

+1 to polarity and phase. Where did terms CPHA and CPOL even come from? We probably sourced them from Arduino examples way back in the day, but I haven't really seen them used outside of that domain. (Also see "mode" etc.)

I feel like "clockSpeed" is more applicable than "frequency" but YMMV.

@Smadattam

This comment has been minimized.

Copy link

Smadattam commented Dec 9, 2017

Hi guys, I'm new here, and am a pretty green programmer. Would it make sense to create a constant titled MHz (or some other descriptive name) instead, and set it equal to 1000x1000? That way the expression could become "4*MHz" which could be even more readable.

arushi019 added a commit to arushi019/t2-docs that referenced this issue Jul 31, 2018

arushi019 added a commit to arushi019/t2-docs that referenced this issue Aug 20, 2018

@arushi019

This comment has been minimized.

Copy link
Contributor

arushi019 commented Aug 23, 2018

Hi @Frijol ! Can the issue be closed now that the pull request has been merged? Or is there something else that needs to be done?

@Frijol

This comment has been minimized.

Copy link
Member

Frijol commented Sep 10, 2018

Yes, thanks for noticing!

@Frijol Frijol closed this Sep 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.