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.

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

Running LoRaWAN Network Server decoupled from IoT Edge #1746

Merged
merged 25 commits into from
Jul 4, 2022
Merged

Conversation

danigian
Copy link
Collaborator

@danigian danigian commented Jun 29, 2022

PR for issue #1553

What is being addressed

This PR is bringing into dev branch all the work related to Decouple LNS from Edge milestone

Validation:

bastbu and others added 16 commits May 17, 2022 08:56
* Adding EdgeDeviceGetter and tests

* Formatting and changing accessibility

* Locking on redis cache

* Adding to DI

* Introducing cancellation token

* Logging an error if we were not able to update redis

* Adding prefix

* Missed one statement

* Adding timeout logic
* Introduce CLOUD_DEPLOYMENT variable #1627

* When not running as Edge module, ENABLE_GATEWAY can't be set to true #1628

* Make ProcessingDelayInMilliseconds configurable from environment variables #1629

* Fix unit tests

* Add PR comment changes from bastbu and danigian
…dis NuGet Package (#1693)

* Adding StackExchange.Redis NuGet package

* Adding RedisConnectionString environment variable to configuration

* Adding a configuration unit test

* Redis subscriber implementation (#1699)

* Fixing injection

* Making analyzer happy about null reference exceptions

* Fixing UTs

* Additional UT fix

* Update LoRaEngine/modules/LoRaWanNetworkSrvModule/LoRaWan.NetworkServer/NetworkServerConfiguration.cs

Co-authored-by: Bastian Burger <[email protected]>

Co-authored-by: Bastian Burger <[email protected]>
* Providing alternatives to IOTEDGE_* environment variables

* Falling back without if else
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Bastian Burger <[email protected]>
TheoryDataFactory.From(
(string.Empty, "Unable to parse Json when attempting to close"),
("null", "Missing payload when attempting to close the"),
(JsonSerializer.Serialize(new { DevEui = (string)null, Fport = 1 }), "DevEUI missing"),

Check warning

Code scanning / CodeQL

Useless upcast

There is no need to upcast from [null](1) to [String](2) - the conversion can be done implicitly.
{
_ = loggingBuilder.ClearProviders();
var logLevel = int.TryParse(NetworkServerConfiguration.LogLevel, NumberStyles.Integer, CultureInfo.InvariantCulture, out var logLevelNum)
? (LogLevel)logLevelNum is var level && Enum.IsDefined(typeof(LogLevel), level) ? level : throw new InvalidCastException()

Check warning

Code scanning / CodeQL

Constant condition

Condition always evaluates to 'false'.
{
_ = loggingBuilder.ClearProviders();
var logLevel = int.TryParse(NetworkServerConfiguration.LogLevel, NumberStyles.Integer, CultureInfo.InvariantCulture, out var logLevelNum)
? (LogLevel)logLevelNum is var level && Enum.IsDefined(typeof(LogLevel), level) ? level : throw new InvalidCastException()

Check warning

Code scanning / CodeQL

Constant condition

Pattern never matches.
Tests/Unit/NetworkServer/ConfigurationTest.cs Fixed Show fixed Hide fixed
Tests/Unit/NetworkServer/ConfigurationTest.cs Fixed Show fixed Hide fixed
{
_ = loggingBuilder.ClearProviders();
var logLevel = int.TryParse(NetworkServerConfiguration.LogLevel, NumberStyles.Integer, CultureInfo.InvariantCulture, out var logLevelNum)
? (LogLevel)logLevelNum is var level && Enum.IsDefined(typeof(LogLevel), level) ? level : throw new InvalidCastException()

Check warning

Code scanning / CodeQL

Useless assignment to local variable

This assignment to [level](1) is useless, since its value is never read.
Comment on lines +70 to +77
if (isCloudDeployment && !shouldSetRedisString)
{
_ = Assert.Throws<InvalidOperationException>(lnsConfigurationCreation);
}
else
{
_ = lnsConfigurationCreation();
}

Check notice

Code scanning / CodeQL

Missed ternary opportunity

Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
@danigian danigian linked an issue Jun 29, 2022 that may be closed by this pull request
4 tasks
Tests/Simulation/SimulatedCloudTests.cs Fixed Show fixed Hide fixed
Comment on lines +118 to +121
catch (Exception)
{
// Dispose all basics stations
}

Check notice

Code scanning / CodeQL

Generic catch clause

Generic catch clause.
@codecov-commenter
Copy link

codecov-commenter commented Jul 1, 2022

Codecov Report

Merging #1746 (85fffa4) into dev (3de221a) will increase coverage by 0.16%.
The diff coverage is 93.15%.

@@            Coverage Diff             @@
##              dev    #1746      +/-   ##
==========================================
+ Coverage   86.58%   86.75%   +0.16%     
==========================================
  Files         236      243       +7     
  Lines        9279     9480     +201     
==========================================
+ Hits         8034     8224     +190     
- Misses       1245     1256      +11     
Impacted Files Coverage Δ
LoRaEngine/LoraKeysManagerFacade/FacadeStartup.cs 0.00% <0.00%> (ø)
...gine/LoraKeysManagerFacade/ServiceClientAdapter.cs 0.00% <0.00%> (ø)
LoRaEngine/LoraKeysManagerFacade/ClearLnsCache.cs 86.11% <86.11%> (ø)
...BasicsStation/BasicsStationNetworkServerStartup.cs 60.15% <89.85%> (+3.48%) ⬆️
...csStation/ModuleConnection/ModuleConnectionHost.cs 91.35% <90.00%> (-2.96%) ⬇️
...ade/FunctionsBundler/DeduplicationExecutionItem.cs 94.80% <93.33%> (+0.86%) ⬆️
...RaEngine/LoraKeysManagerFacade/EdgeDeviceGetter.cs 96.36% <96.36%> (ø)
...dule/LoRaWan.NetworkServer/LnsRemoteCallHandler.cs 96.49% <96.49%> (ø)
...ule/LoRaWan.NetworkServer/LnsRemoteCallListener.cs 96.55% <96.55%> (ø)
...ndCloudToDeviceMessage/SendCloudToDeviceMessage.cs 84.03% <96.66%> (+2.66%) ⬆️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3de221a...85fffa4. Read the comment docs.

@danigian danigian marked this pull request as ready for review July 1, 2022 08:16
@danigian danigian added the fullci Run full continuous integration label Jul 1, 2022
@danigian danigian temporarily deployed to CI_AZURE_ENVIRONMENT July 1, 2022 08:16 Inactive
@danigian danigian temporarily deployed to CI_AZURE_ENVIRONMENT July 1, 2022 08:16 Inactive
@danigian danigian temporarily deployed to CI_ALL_IN_ONE_ARM_GATEWAY July 1, 2022 08:18 Inactive
@danigian danigian temporarily deployed to CI_EFLOW_AZURE_VM July 1, 2022 08:18 Inactive
@danigian danigian temporarily deployed to CI_CONCENTRATOR_EFLOW_ARM32 July 1, 2022 08:18 Inactive
@danigian danigian temporarily deployed to CI_AZURE_ENVIRONMENT July 1, 2022 08:19 Inactive
@danigian danigian temporarily deployed to CI_AZURE_ENVIRONMENT July 1, 2022 08:21 Inactive
@github-actions github-actions bot added CloudDeploymentTest SensorDecodingTest Indicate SensorDecodingTest passed OTAAJoinTest Indicate OTAAJoinTest passed ABPTest Indicate ABPTest passed MacTest Indicate MacTest passed ClassCTest Indicate ClassCTest passed C2DMessageTest Indicate C2DMessageTest passed labels Jul 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ABPTest Indicate ABPTest passed C2DMessageTest Indicate C2DMessageTest passed ClassCTest Indicate ClassCTest passed CloudDeploymentTest fullci Run full continuous integration MacTest Indicate MacTest passed OTAAJoinTest Indicate OTAAJoinTest passed SensorDecodingTest Indicate SensorDecodingTest passed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Decouple LNS from IoT Edge
7 participants