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

⏳ add helper class for ESP32 pins #1288

Closed
wants to merge 2 commits into from
Closed

⏳ add helper class for ESP32 pins #1288

wants to merge 2 commits into from

Conversation

Dweaver309
Copy link
Contributor

@Dweaver309 Dweaver309 commented Apr 20, 2019

Description

Motivation and Context

How Has This Been Tested?

Screenshots

Types of changes

  • Improvement (non-breaking change that improves a feature, code or algorithm)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Signed-off-by: GITHUB_USER <GITHUB_USER_EMAIL>

Please check package.nuspec file for errors.
@nfbot
Copy link
Member

nfbot commented Apr 20, 2019

Hi @Dweaver309,

I'm nanoFramework bot.
Thank you for your contribution!

A human will be reviewing it shortly. 😉

@CLAassistant
Copy link

CLAassistant commented Apr 20, 2019

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@AdrianSoundy AdrianSoundy left a comment

Choose a reason for hiding this comment

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

I am not sure what you are trying to achieve with this class. Its only useful if you only ever just use the default mapping. Then i am not sure where you would use it.
All you are doing here is listing what the default pins are currently set for the ESP32 on boot.
There is no fixed assignment of pins to a function with ESP32 which is unlike the STM32 devices which have fixed functions assigned to pins and then you select which function to use on a pin.

For example you can assign whatever pins you like for most devices(e.g SPI1, SPI2 etc) using Hardware.Esp32.Configuration.SetPinFunction(Int32, DeviceFunction) which changes the mapping for when the device is referenced. The SPI1 pins are where they are because the Wrover board display uses those pins. Another board may use different pins for SPI bus.

I think the ADC pins are the exception to this.

@Dweaver309
Copy link
Contributor Author

Dweaver309 commented Apr 23, 2019 via email

@josesimoes josesimoes added Series: ESP32 Everything related specifically with ESP32 series targets Status: under review labels Apr 23, 2019
@josesimoes
Copy link
Member

@AdrianSoundy I was the one suggesting @Dweaver309 to add this, following what is there in STM32.

I understand that EPS32 pins can be configured on-the-fly. The same happens (with less flexibility) on STM32. The point here is to have a mapping with more friendly names for the pins when one is coding.

Equivalent helper classes exist for STM32 and they are useful, specially for developers that are jumping in.

Considering the above, do you have any suggestions on making this more "developer friendly"? 😉

@josesimoes
Copy link
Member

@Dweaver309 we are giving this some thought internally and thinking on how this pin remapping can be improved even further.

@josesimoes josesimoes changed the title Develop dweaver309 ⏳ add helper class for ESP32 pins Apr 24, 2019
@Dweaver309
Copy link
Contributor Author

Dweaver309 commented Apr 24, 2019 via email

@AdrianSoundy
Copy link
Member

Started adding this to Hardware.Esp32

@AdrianSoundy
Copy link
Member

AdrianSoundy commented Jun 27, 2019

I have been looking at this but there is a problem.
The pin assignments are not fixed so you can't have a simple constant for the definition of say a SPI1.Mosi pin which means you end up with a function for every pin as it has to get the current pin assignment. This will add a lot of mostly unused code to the assembly.

So what i have decided to just limit the change to a new function :-
public static int GetFunctionPin(DeviceFunction function)

This will return the current pin assignment for any device pin which was missing from the assembly.
Also i have change it to directly be able to set the mapping instead of using the alternate function on a gpio pin.

I will extend it to include ADC which is currently missing, but these are fixed pins.

@josesimoes
Copy link
Member

Got you. If you think that's the best way to tackle this go for it! 👍

@AdrianSoundy
Copy link
Member

AdrianSoundy commented Jul 2, 2019

This can be closed once Esp32 hardware update #1392 is closed and the update to
lib-nanoFramework.Hardware.Esp32
@Dweaver309

@josesimoes josesimoes added invalid and removed Series: ESP32 Everything related specifically with ESP32 series targets Status: in progress labels Jul 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants