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

Improvements to setOutPut() #20

Open
sheffieldnikki opened this issue Jun 8, 2022 · 2 comments
Open

Improvements to setOutPut() #20

sheffieldnikki opened this issue Jun 8, 2022 · 2 comments
Assignees
Labels
enhancement New feature or request Seeed_Arduino_AS5600 Label for Seeed_Arduino_AS5600 UAY Unassigned yet

Comments

@sheffieldnikki
Copy link

sheffieldnikki commented Jun 8, 2022

There is a bug where only mode == 0 can set the output mode. mode == 1 is trapped by the first if clause and never written. And a bitwise operations bug because config_status & 0xef can only set OUTS1:0 to 10 (digital PWM) if the register was already 10 or 11, not if it was 00 or 01.

Suggested improvements:

  1. Added function documentation for consistency with rest of library
  2. Bug fix: mode == 1 for analog output now works, removed nested if clause
  3. Removed duplicate readOneByte() call, function now twice as fast
  4. New feature: mode == 2 sets reduced range analog 10-90% (better for some ADCs)
  5. Comments inside function to explain clearing/setting register bits
  6. Shorthand assignment operator &=
  7. Renamed function to setOutput, since "output" is one word
  8. removed lowByte(), since by definition config_status is a single byte
  9. Folded variable declaration into fewer lines
  10. Bug fix: correct bitwise operations for set/clear bits in register
/*******************************************************
  Method: setOutput
  In: 0 for digital PWM
      1 for analog (full range 0-100% of GND to VDD)
      2 for analog (reduced range 10-90%)
  Out: none
  Description: sets output mode in CONF register.
*******************************************************/
void AMS_5600::setOutput(uint8_t mode)
{
  int _conf_lo = _addr_conf+1; // lower byte address
  uint8_t config_status = readOneByte(_conf_lo);
  config_status &= 0b11001111; // bits 5:4 = 00, default
  if (mode == 0) {
    config_status |= 0b100000; // bits 5:4 = 10
  } else if (mode == 2) {
    config_status |= 0b010000; // bits 5:4 = 01
  }
  writeOneByte(_conf_lo, config_status);
}
@Pillar1989
Copy link
Member

@sheffieldnick welcome to PR.

@sheffieldnikki sheffieldnikki closed this as not planned Won't fix, can't repro, duplicate, stale Jun 9, 2022
@sheffieldnikki sheffieldnikki reopened this Jun 9, 2022
@sheffieldnikki
Copy link
Author

@Pillar1989 no PR, but you've got my code so you're welcome to merge it if you want.

Pillar1989 pushed a commit that referenced this issue Aug 25, 2022
Implement Issue "Improvements to detectMagnet() / getMagnetStrength()
#19 opened on 8 Jun by sheffieldnick
Implement Issue "Improvements to setOutPut()
#20 opened on 8 Jun by sheffieldnick"
@MatthewJeffson MatthewJeffson added UAY Unassigned yet Seeed_Arduino_AS5600 Label for Seeed_Arduino_AS5600 labels Oct 9, 2024
@Lesords Lesords self-assigned this Oct 14, 2024
@Lesords Lesords added the enhancement New feature or request label Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Seeed_Arduino_AS5600 Label for Seeed_Arduino_AS5600 UAY Unassigned yet
Projects
Status: Todo
Development

No branches or pull requests

4 participants