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.

Sign up for GitHub

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

Jump to bottom

Compiler warning: unused variable in LoRaMac.h #7807

Merged
merged 1 commit into from
Aug 21, 2018
Merged

Compiler warning: unused variable in LoRaMac.h #7807

merged 1 commit into from
Aug 21, 2018

Conversation

hasnainvirk
Copy link
Contributor

Description

Fixing an unused variable warning

Pull request type

[X] Fix
[ ] Refactor
[ ] Target update
[ ] Feature
[ ] Breaking change

@hasnainvirk
Copy link
Contributor Author

@0xc0170 Please review. Minor compiler warning fix.

@hasnainvirk hasnainvirk mentioned this pull request Aug 16, 2018
Closed
@cmonr
Copy link
Contributor

cmonr commented Aug 16, 2018

@hasnainvirk Please update the PR title and commit to be something a bit more specific than "a minor warning"

@cmonr cmonr self-requested a review August 16, 2018 14:31
@hasnainvirk hasnainvirk changed the title Fixing a minor warning Compiler warning: unused variable in LoRaMac.h Aug 17, 2018
unused variable warning in LoRaMac.h for release build
a7cb32f 
In release builds where MBED_ASSERT macro is syphoned off, we were hitting
an unused variable warning. To mitigate that we could direct the variable
used in MBED_ASSERT to void.
@hasnainvirk
Copy link
Contributor Author

@cmonr How does it look now ? Sorry for the lame message in the first attempt. Was in hurry to leave for home :D

@mattbrown015
Copy link
Contributor

It looks to me like Mutex lock has possibility to fail at runtime (returning status flag) #6872 means a better solution would be to remove the status check.

I haven't considered the discussion is great detail but some people think that Mutex::lock shouldn't return a value and the only reason it does is to maintain backward compatibility.

@cmonr
Copy link
Contributor

cmonr commented Aug 17, 2018

@mattbrown015 This PR is only correcting a unused variable warning. It looks like the discussion in regards to issue #6872 is continuing in this PR: #7423

/morph build

@mbed-ci
Copy link

mbed-ci commented Aug 17, 2018

Build : SUCCESS

Build number : 2831
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7807/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci
Copy link

mbed-ci commented Aug 18, 2018

Exporter Build : SUCCESS

Build number : 2462
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/7807/

@mbed-ci
Copy link

mbed-ci commented Aug 18, 2018

Test : FAILURE

Build number : 2583
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/7807/2583

@hasnainvirk
Copy link
Contributor Author

@cmonr Actually @mattbrown015 is right. osStatus Mutex::lock will soon get deprecated in favour of void Mutex::lock. I am not sure if this PR is worth it. I pushed this PR thinking that the #7423 will take time. Anyway your call to keep the PR or discard it.

@hasnainvirk
Copy link
Contributor Author

@cmonr Do we need to restart the CI ?

@0xc0170
Copy link
Contributor

0xc0170 commented Aug 20, 2018

/morph test

@mbed-ci
Copy link

mbed-ci commented Aug 21, 2018

Test : SUCCESS

Build number : 2593
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/7807/2593

@NirSonnenschein
Copy link
Contributor

/morph uvisor-test

@cmonr cmonr merged commit bf26cbf into ARMmbed:master Aug 21, 2018
pan- pushed a commit to pan-/mbed that referenced this pull request Aug 22, 2018
Merge pull request ARMmbed#7807 from hasnainvirk/warning_fix
fae3f65 
Compiler warning: unused variable in LoRaMac.h
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cmonr cmonr cmonr approved these changes

@0xc0170 0xc0170 0xc0170 approved these changes

Assignees
No one assigned
Labels
release-version: 5.9.6
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@hasnainvirk @cmonr @mattbrown015 @mbed-ci @0xc0170 @NirSonnenschein