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.

Sign up for GitHub

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

Jump to bottom

Fixed length pin string names #180

Closed
PeteLawler opened this issue Oct 24, 2017 · 7 comments
Closed

Fixed length pin string names #180

PeteLawler opened this issue Oct 24, 2017 · 7 comments
Labels
enhancement

Comments

@PeteLawler
Copy link
Contributor

So I'm of the habit that when I'm dealing with numbers or strings of generally a specified length, that I include leading 0's if needs be.

Thus in a numbering scheme where we go from 0 to 99, I'll tend to use 00, 01, 02, 03... etc. up to 99. This can make, if I need to, string manipulation easier.

After a month or two of tracking down a 'bug', I've finally nailed it. Can you see where I'm going?

Bug:
SystemError: returned NULL without setting an error

Cause:
GPIO.setup("P8_07", OUT )
Fix:
GPIO.setup("P8_7", OUT )

I'm wondering if, apart from within source, this should be noted on the README, or whether the source could be updated. I'm not sure about the latter because I don't quite know about external compatibilies you may be wishing to maintain. I'm happy to do any diff, just eager for any guidance....

I can't believe... ok I can... that it took this long to track down.
@RobertCNelson
Copy link
Contributor

@PeteLawler i know @jadonk wants to see these values converted to a mixed case option, with leading/no-leading 0's and mixed symbols: '.','_', etc..

https://github.com/adafruit/adafruit-beaglebone-io-python/blob/master/source/common.c#L73-L261

and @pdp7 is working on splitting that single table up into a per board table.

Regards,

@PeteLawler
Copy link
Contributor Author

@RobertCNelson
Fair enough.
I'll see about whether I can come up with some wording for the README or some error handling, or whether I'd prefer just to cower in the corner from guilt and shame and pretend none of the past month of bug hunting ever happened 😭

@pdp7
Copy link
Collaborator

pdp7 commented Oct 24, 2017 •
edited

@PeteLawler that does seem awkward to have P8_7 in the table instead of P8_07. I suppose one way to prevent this issue is to accept both P8_7 and P8_07.

For reference, P8_07 does seem like the best choice as that is what pinmux helper driver is using:
/sys/devices/platform/ocp/ocp:P8_07_pinmux

@RobertCNelson
Copy link
Contributor

@pdp7 and config-pin already does this internal pin name fixup:

https://github.com/beagleboard/bb.org-overlays/blob/master/tools/beaglebone-universal-io/config-pin#L1127-L1164

Probably steal some of that syntax. ;)

Regards,

@PeteLawler
Copy link
Contributor Author

Ah great I was pretty sure I'd seen that nomenclature somewhere but after this adventure I really wondered. Had a brief squizz at the SRM, didn't see anything, then went back to coffee.
I can understand where @jadonk is coming from, though. Sticking with SRM nomenclature throughout would be best, no? Or maybe the SRM is wrong and should use fixed length strings.😁

@RobertCNelson
Copy link
Contributor

Considering the audience, i don't mind, P8_01/P8_1/P8.01/P8.1.. it's just implementing that in python.. ;) bash is easy..

pdp7 added a commit that referenced this issue Nov 28, 2017
@pdp7
Clarify there is no 0 prefix for pin lables (#180)
0e593a1 
Please note that there is no '0' prefix for the pin numbers. For example, pin 7 on header P8 is "P8_7"
@pdp7
Copy link
Collaborator

pdp7 commented Nov 28, 2017 •
edited

I've added documentation to the README that there is no '0' prefix for the pin numbers.

I'm going to close this issue but would be happy to review any pull requests that add logic for alternate formats for pin labels.

@pdp7 pdp7 closed this as completed Nov 28, 2017
pdp7 added a commit that referenced this issue Dec 1, 2017
@pdp7
create release 1.0.9
f64382b 
Features:
  * Issue #194: Encoder position cannot be set
  * PR #205: Encoder: add support for reading/writing sysfs attributes

Fixes:
  * Issue #198: use https for DEFAULT_URL in distribute_setup.py
  * Issue #197: Fix leak of pwm enable file descriptor
  * Issue #189: Fix seg fault of PWM in Python 3.6
  * Issue #180: Clarify there is no 0 prefix for pin lables
  * PR #201: Encoder: do kernel check, PEP8 cleanup
  * PR #202: Encoder: corrected kernel check logic
  * PR #207: Encoder: improved usage adocumentation
  * PR #210: Encoder: fix sysfs import, make code Python 3 compatible
  * PR #212: Encoder: fix Python 3 compatibility
  * PR #213: Encoder: fix frequency calculation from period

shortlog:

* David Planella (18):
  * Encoder: initialize only the given channel
  * Sync from master
  * Encoder: do kernel check, PEP8 cleanup
  * Encoder: added sysfs module
  * Encoder: use sysfs to write QEP attributes
  * Encoder: corrected kernel check logic
  * Merge pull request #2 from adafruit/master
  * Encoder: convert get/set methods to properties, update apidoc strings
  * Encoder: updated README
  * Encoder: add README apt install clarification
  * Encoder: copyright assignment note, updated comments
  * Encoder: added usage notes
  * Encoder: improved usage documentation
  * Encoder: minor fix to usage example
  * Encoder: added a note about permissions
  * Encoder: switched sysfs to be a relative import compatible with Python 2 and 3
  * Encoder: use items() instead of iteritems() to be Python 3 compatible
  * Encoder: fix frequency getter

* Drew Fustini (18):
  * use https for DEFAULT_URL in distribute_setup.py (#198)
  * fix except syntax for Python 3
  * use dict.items() instead of dict.iteritems() for Python 3
  * fix error in set_brightness()
  * close enable_fd when stopping PWM output (#197)
  * Merge pull request #199 from dplanella/patch-1
  * Fix leak of pwm enable file descriptor (#197)
  * Merge pull request #201 from dplanella/encoder-cleanup
  * remove test_rotary.py as not valid for pytest
  * Fix seg fault of PWM in Python 3.6 (#189)
  * Merge pull request #202 from dplanella/patch-2
  * Clarify there is no 0 prefix for pin lables (#180)
  * Merge pull request #205 from dplanella/encoder-sysfs
  * assign copyright for new file to Adafruit Industries
  * Add bash scripts to help install and test
  * Merge pull request #212 from dplanella/patch-4
  * Merge pull request #207 from dplanella/patch-3
  * Merge pull request #213 from dplanella/fix-encoder-frequency

Signed-off-by: Drew Fustini <drew@pdp7.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@RobertCNelson @PeteLawler @pdp7