Fix GPS and LEDs on ThinkNode M3#2887
Open
spinda wants to merge 1 commit into
Open
Conversation
GPS support was implemented incorrectly for the ThinkNode M3. The reset pin was being driven when it should be left floating for this unit. The enable pin also wasn't being picked up by `MicroNMEALocationProvider` because of a mismatch in constant naming conventions. I did a general cleanup of the GPS and ThinkNode M3 bring-up code so that constant names line up and "*_ACTIVE" constants are used consistently vs hardcoding `HIGH`/`LOW`. After making these changes, serial data immediately starts streaming in from the NMEA on boot and GPS detection just works. LED handling was also not quite right for the ThinkNode M3. The LoRa TX LED was being driven high to turn it on when it should actually be driven low. I changed the code to use the `LED_STATE_ON` constant and also added a little code to the shutdown path to properly make sure that all LEDs are turned off. Tested and confirmed working on real hardware. Resolves both meshcore-dev#1864 and meshcore-dev#2879.
Collaborator
|
Oh that's great ... I did the initial port quick and then I forgot the unit in France (and it is still there). I remember I had issues with gps, it's great you could sort this out ... I see there is still a lot of mess in symbols for the GPSes on different boards, could you verify how your changes affect other variants ? We usually prefer keeping the PRs focused and as small as possible ... I think here it would be better to focus on making the M3's peripheral work and in another PR do some refactoring on symbol names (there is really a need for that part, and I agree for the bad choice of GPS_RESET_FORCE). You can mention me in the other PR so I review and merge it. Thanks. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GPS support was implemented incorrectly for the ThinkNode M3. The reset pin was being driven when it should be left floating for this unit. The enable pin also wasn't being picked up by
MicroNMEALocationProviderbecause of a mismatch in constant naming conventions. I did a general cleanup of the GPS and ThinkNode M3 bring-up code so that constant names line up and "*_ACTIVE" constants are used consistently vs hardcodingHIGH/LOW. After making these changes, serial data immediately starts streaming in from the NMEA on boot and GPS detection just works.LED handling was also not quite right for the ThinkNode M3. The LoRa TX LED was being driven high to turn it on when it should actually be driven low. I changed the code to use the
LED_STATE_ONconstant and also added a little code to the shutdown path to properly make sure that all LEDs are turned off.Tested and confirmed working on real hardware.
Resolves both #1864 and #2879.