Initial version of nanoFramework.Hardware.Esp32 #1
Conversation
|
Hi @AdrianSoundy, I'm nanoFramework bot. A human will be reviewing it shortly. 😉 |
|
|
source/Errors.cs
Outdated
| /// <summary> | ||
| /// Encapsulates the ESP32 native errors | ||
| /// </summary> | ||
| public class Errors |
There was a problem hiding this comment.
Considering that this class doesn't hold anything else except the enum, I would remove it shortening the path to reach it.
source/Errors.cs
Outdated
| // <summary> | ||
| // Espressif error codes | ||
| // </summary> | ||
| public enum Esp : Int32 |
There was a problem hiding this comment.
Perhaps something more meaningful like EspNativeError...
source/Errors.cs
Outdated
| /// <summary> | ||
| /// Memory allocation failed error | ||
| /// </summary> | ||
| ESP_ERR_NO_MEM = 0x101, |
There was a problem hiding this comment.
Starting all enum items echoing the enum name is more a C/C++ naming convention/style.
Removing the ESP_ERR_ from them makes it more readable. And perhaps using camel case in the names helps on that too.
There was a problem hiding this comment.
All good suggestions
| </Dependency> | ||
| </ItemGroup> | ||
| <PropertyGroup Label="Globals"> | ||
| <ProjectGuid>96eaba11-6a33-454d-a7b2-b96ca5617e8c</ProjectGuid> |
There was a problem hiding this comment.
Please replace this GUID, it's been used in the equivalent Nuget project in mscorlib repo
There was a problem hiding this comment.
Also changed the DELIVERABLES one as well, just in case.
I thought there would be a issue with GUIDs, especially starting from copied projects.
| @@ -0,0 +1,74 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
There was a problem hiding this comment.
Several fixes here:
- correct the Nuget name to nanoFramework.Hardware.ESP32
- the include files bellow should be referring to the ones from this project instead of Gpio ones
- the description should be descriptive for this package
source/Sleep.cs
Outdated
| /// <summary> | ||
| /// Touch pad channel 0 is GPIO4 | ||
| /// </summary> | ||
| TOUCH_PAD_NUM0 = 0, |
source/Sleep.cs
Outdated
| /// </summary> | ||
| /// <param name="time"></param> | ||
| /// <returns></returns> | ||
| public static Errors.Esp EnableWakupByTimer(TimeSpan time) |
There was a problem hiding this comment.
Type in method name: missing the e
source/Sleep.cs
Outdated
| /// 0,2,4,12->15,25->27,32->39</param> | ||
| /// <param name="level"></param> | ||
| /// <returns></returns> | ||
| public static Errors.Esp EnableWakupByPin(WakeupGpioPin pin, int level) |
There was a problem hiding this comment.
Type in method name: missing the e
There was a problem hiding this comment.
Ok got all the Wakup
| </Content> | ||
| </ItemGroup> | ||
| <ItemGroup> | ||
| <Dependency Include="nanoFramework.CoreLibrary"> |
There was a problem hiding this comment.
The dependencies descriptions bellow are pointing to outdated versions.
| <Folder Include="lib\" /> | ||
| </ItemGroup> | ||
| <ItemGroup> | ||
| <Dependency Include="nanoFramework.CoreLibrary"> |
There was a problem hiding this comment.
The dependencies descriptions bellow are pointing to outdated versions.
- Was using the same as the other Nuget project
No description provided.