noopkat / avrgirl-arduino

:girl: :pager: A NodeJS library for flashing compiled sketch files to Arduino microcontroller boards.
MIT License
506 stars 129 forks source link

Provide a static helper to list known boards #123

Closed whitfin closed 7 years ago

whitfin commented 7 years ago

This PR is a resolution for #122.

It exposes a new static method named listKnownBoards using the keys from the byName object which is keyed by the board name. I went with this over listBoards as I figured that might be useful in future as an instance method which also includes things such as custom boards.

I also added aliasing to remove the duplication mentioned in the stream today, and verified that both names come back appropriately in the resulting call to listKnownBoards. It might be that we don't want the aliases to come back in this list though, if so just let me know and I can tweak it to omit them.

Here's an example output:

> avr.listKnownBoards()
[ 'uno',
  'micro',
  'imuduino',
  'leonardo',
  'arduboy',
  'feather',
  'little-bits',
  'blend-micro',
  'nano',
  'duemilanove168',
  'duemilanove328',
  'tinyduino',
  'tinduino',
  'bqZum',
  'mega',
  'adk',
  'sf-pro-micro',
  'pro-mini',
  'qduino',
  'pinoccio',
  'lilypad-usb',
  'yun' ]

Feel free to make any changes you think should be made, or just let me know and I can make them as needed :D!

whitfin commented 7 years ago

As a side note, the byName property is a little redundant and could probably be removed in future in favour of module.exports = boardLookupTable().

coveralls commented 7 years ago

Coverage Status

Coverage increased (+1.04%) to 45.161% when pulling 58a6c855fc2d16b311d338591df8a0d457f82522 on zackehh:issue-122 into 2e4ddc417be1fb461bc960d5082b1255611e10c9 on noopkat:master.

noopkat commented 7 years ago

@zackehh yayyy!! Thank you for turning this around so quick! I agree with you on the redundant byName prop - I used to have a couple of different mappings that were removed. Could you be so kind as to refactor that out (using your suggested approach) as well? Would clean things up nicely! ☺️

noopkat commented 7 years ago

also - I am going to mull over the alias filtering for the outputted list. Not sure what is the best call, but I'll let you know later today πŸ‘

whitfin commented 7 years ago

@noopkat sure thing, I'll also push the byName prop change when you decide so as to reduce the commit noise πŸ”” Have a great Sunday!

noopkat commented 7 years ago

@zackehh hai so I think we should filter out aliases from the returned list, if it's not too much trouble. I know it's more work for you - apologies

whitfin commented 7 years ago

@noopkat no problemo! I've pushed that change, let me know if you need anything else changing!

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.7%) to 44.84% when pulling 8e2b694f0062ebb804f23d1fea62384953f63b8a on zackehh:issue-122 into 2e4ddc417be1fb461bc960d5082b1255611e10c9 on noopkat:master.

noopkat commented 7 years ago

bitmoji

noopkat commented 7 years ago

thanks again, @zackehh ! πŸ™‡β€β™€οΈ