Skip to content

Conversation

@putridparrot
Copy link
Contributor

@putridparrot putridparrot commented Nov 5, 2022

Description

Reversed the logic for setting the PowerLed as it seems incorrect.

Motivation and Context

Testing the PowerLed property, so I found that M5Core2.PowerLed = true turns off the PowerLed and a value of false turns it on. I expected this would be the opposite way around, i.e. true PowerLed on and false PowerLed off.

I'm fairly new to electronics and the M5Core2 in particular, so I'm not sure where the original code came from for this, i.e. M5Core2 spec or datasheet. So whilst this change works the original code did look like it makes perfect sense, and it would be good if somebody has the spec sheet for this to double check the logic.

How Has This Been Tested?

On the M5Core2 I originally set used the code M5Core2.PowerLed = true expecting the PowerLed to turn on. It didn't. So I switched to M5Core2.PowerLed = false and the PowerLed turned on.

I then looked at the code and tested again but this time by calling M5Core2.Power.Pwm1DutyCycleSetting1 and M5Core2.Power.Pwm1DutyCycleSetting2 directly to see that the logic that this worked correctly. I then made the changes to switch the logic in the nanoFramework.M5Stack assembly. I rebuilt the project and dropped the binary and support files into the package location of the original downloaded NuGet Package and run in my little test app. The PowerLed now seems to turn on/off correctly, i.e. PowerLed = true, led is on and false its off.

Screenshots

Types of changes

  • Improvement (non-breaking change that improves a feature, code or algorithm)
  • Bug fix (non-breaking change which fixes an issue with code or algorithm)
  • New feature (non-breaking change which adds functionality to code)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Config and build (change in the configuration and build system, has no impact on code or features)
  • Dependencies (update dependencies and changes associated, has no impact on code or features)
  • Unit Tests (add new Unit Test(s) or improved existing one(s), has no impact on code or features)
  • Documentation (changes or updates in the documentation, has no impact on code or features)

Checklist:

  • My code follows the code style of this project (only if there are changes in source code).
  • My changes require an update to the documentation (there are changes that require the docs website to be updated).
  • I have updated the documentation accordingly (the changes require an update on the docs in this repo).
  • I have read the CONTRIBUTING document.
  • I have tested everything locally and all new and existing tests passed (only if there are changes in source code).
  • I have added new tests to cover my changes.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 5, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@nfbot nfbot added the Type: bug Something isn't working label Nov 5, 2022
Copy link
Member

@josesimoes josesimoes left a comment

Choose a reason for hiding this comment

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

LGTM.
Someone else has already mentioned this on Discord and I trust you've teste it, so... 👍🏻

@josesimoes josesimoes changed the title The logic for the PowerLed appears to be reversed Fix M5Core2 power LED on/off (was reversed) Nov 5, 2022
@josesimoes josesimoes merged commit ab76652 into nanoframework:main Nov 5, 2022
@nfbot
Copy link
Member

nfbot commented Nov 5, 2022

@putridparrot thank you again for your contribution! 🙏😄

.NET nanoFramework it's all about community involvement and no contribution is too small.
We would like to invite you to join the project Contributors list.

Please edit it and add an entry with your GitHub user in the appropriate location (names sorted alphabetically):

  <tr>
    <td><img src="https://github.com/putridparrot.png?size=50&" height="50" width="50" ></td>
    <td><a href="https://github.com/putridparrot">Mark Timmings</a></td>
  </tr>

(feel free to adjust your name, if it's not correct)

@putridparrot
Copy link
Contributor Author

Yes, I did check that somebody else had seen the same on the Discord channel and yes I've tested, in fact I have it running next to me at the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Breaking-Change Type: bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants