-
Notifications
You must be signed in to change notification settings - Fork 3
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
Bug fix and better logging #26
Conversation
oliviamiller
commented
Feb 14, 2025
- Added mutex around data file, John hit an issue where two entries for the same Dev EUI were written to the file.
- Parsing confirmed data up messages instead of erroring, will add implementation for sending the confirmation downlink in a new PR
- Adding more info to a few of the logs for easier debugging
gateway/gateway.go
Outdated
@@ -371,8 +372,21 @@ func (g *gateway) handlePacket(ctx context.Context, payload []byte) { | |||
return | |||
} | |||
g.updateReadings(name, readings) | |||
case 0x80: | |||
g.logger.Infof("received confirmed data uplink") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for adding these! In a future pr we should move the "received" logs down to debug I think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on a similar note, might be worth adding a "join accepted" log as well
@@ -458,10 +473,12 @@ func (g *gateway) DoCommand(ctx context.Context, cmd map[string]interface{}) (ma | |||
// searchForDeviceInfoInFile searhces for device address match in the module's data file and returns the device info. | |||
func (g *gateway) searchForDeviceInFile(devEUI []byte) (*deviceInfo, error) { | |||
// read all the saved devices from the file | |||
g.dataMu.Lock() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not defer the unlock right after?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You've got a case where you return and don't unlock in the error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for catching, switching to defer