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

Reorder pins in bi_pins_with_names to be correct #1858

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

will-v-pi
Copy link

Alternative to #1791 - instead of asserting when the order is incorrect, reorder the names so they are in the correct order for the pin mask

Requires more pre-processor stuff, but doesn't break existing code with the pins in the wrong order, so pico-examples/524 would no longer be required

Has been tested using the following python code to generate all permutations of bi_decls which are then copied and compiled into a binary, and then checked with picotool info to ensure pins are all named correctly

import itertools

pins = [
    'PICO_DEFAULT_SPI_RX_PIN, "SPI RX"',
    'PICO_DEFAULT_SPI_TX_PIN, "SPI TX"',
    'PICO_DEFAULT_SPI_SCK_PIN, "SPI SCK"',
    'PICO_DEFAULT_SPI_CSN_PIN, "SPI CS"'
]

pinperms = itertools.permutations(pins, 2)
for p in pinperms:
    print(f"bi_decl(bi_2pins_with_names({p[0]}, {p[1]}));")

pinperms = itertools.permutations(pins, 3)
for p in pinperms:
    print(f"bi_decl(bi_3pins_with_names({p[0]}, {p[1]}, {p[2]}));")

pinperms = itertools.permutations(pins, 4)
for p in pinperms:
    print(f"bi_decl(bi_4pins_with_names({p[0]}, {p[1]}, {p[2]}, {p[3]}));")

@will-v-pi will-v-pi added this to the 2.0.1 milestone Aug 21, 2024
@lurch
Copy link
Contributor

lurch commented Aug 21, 2024

ZOMG! 🤯 As long as you've tested this, I'll assume it's okay 😉

Although I guess your tests could be simplified to use e.g. pinperms = itertools.permutations(pins[:2], 2) instead?

@kilograham
Copy link
Contributor

Yeah, it is a bit scary; i was wondering if we should do a recursive implementation, but

  1. that means splitting out a separate macro for the string part
  2. I don't know that it is significantly simpler

So unless anyone has any fantastic ideas i say we take this as is ;-)

@lurch
Copy link
Contributor

lurch commented Aug 21, 2024

So unless anyone has any fantastic ideas i say we take this as is ;-)

https://github.com/svpv/qsort 🤣

Copy link
Contributor

@kilograham kilograham left a comment

Choose a reason for hiding this comment

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

I'm gonna believe you!

@kilograham kilograham merged commit 9b16c79 into raspberrypi:develop Aug 22, 2024
1 check passed
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.

3 participants