Skip to content

Project Type Update & Logging Interface #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.

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

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Project Type Update & Logging Interface #3

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f3828ea
Logger interfaces has been implemented also Diagnostics loggers have …
furkanvarol Mar 10, 2015
c0423c2
missing part of FromScanData method
furkanvarol Mar 10, 2015
850f5b0
Beacon and BeaconParser has been adapter to LogManager (TODOs has bee…
furkanvarol Mar 10, 2015
66c58fa
fixed on LogManagers predefined logger factory var type also a defaul…
furkanvarol Mar 11, 2015
93d3b9c
Portable Class Library has been changed with Class Library (Library P…
furkanvarol Mar 12, 2015
17c0f0d
UnitTests for logging package
furkanvarol Mar 12, 2015
1ec58d2
bug fix on WarningDiagnosticsLoggerFactory
furkanvarol Mar 12, 2015
071e902
.Net version has been dropped from 4.5 to 4.0
furkanvarol Mar 12, 2015
010943f
VerboseLoggingEnabled has been renamed to DebugLoggingEnabled
furkanvarol Apr 3, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 21 additions & 6 deletions AltBeacon.Library/AltBeacon.Library.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,16 @@
<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props')" />
<PropertyGroup>
<MinimumVisualStudioVersion>11.0</MinimumVisualStudioVersion>
<Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
<Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
<ProjectGuid>{CB062D54-BA72-4AE5-9E1B-6E4A2CA68C30}</ProjectGuid>
<OutputType>Library</OutputType>
<AppDesignerFolder>Properties</AppDesignerFolder>
<RootNamespace>AltBeacon</RootNamespace>
<AssemblyName>AltBeacon.Library</AssemblyName>
<TargetFrameworkVersion>v4.5</TargetFrameworkVersion>
<TargetFrameworkProfile>Profile7</TargetFrameworkProfile>
<TargetFrameworkVersion>v4.0</TargetFrameworkVersion>
<FileAlignment>512</FileAlignment>
<ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
<TargetFrameworkProfile />
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
<DebugSymbols>true</DebugSymbols>
Expand All @@ -33,7 +31,14 @@
<WarningLevel>4</WarningLevel>
</PropertyGroup>
<ItemGroup>
<!-- A reference to the entire .NET Framework is automatically included -->
<Reference Include="System" />
<Reference Include="System.Core" />
<Reference Include="System.Numerics" />
<Reference Include="System.Xml.Linq" />
<Reference Include="System.Data.DataSetExtensions" />
<Reference Include="Microsoft.CSharp" />
<Reference Include="System.Data" />
<Reference Include="System.Xml" />
</ItemGroup>
<ItemGroup>
<Compile Include="Properties\AssemblyInfo.cs" />
Expand All @@ -44,8 +49,18 @@
</Compile>
<Compile Include="src\AltBeacon\Beacon\Identifier.cs" />
<Compile Include="src\AltBeacon\Bluetooth\BluetoothDevice.cs" />
<Compile Include="src\AltBeacon\Logging\BaseDiagnosticsLogger.cs" />
<Compile Include="src\AltBeacon\Logging\EmptyLogger.cs" />
<Compile Include="src\AltBeacon\Logging\EmptyLoggerFactory.cs" />
<Compile Include="src\AltBeacon\Logging\ILogger.cs" />
<Compile Include="src\AltBeacon\Logging\ILoggerFactory.cs" />
<Compile Include="src\AltBeacon\Logging\LogManager.cs" />
<Compile Include="src\AltBeacon\Logging\VerboseDiagnosticsLogger.cs" />
<Compile Include="src\AltBeacon\Logging\VerboseDiagnosticsLoggerFactory.cs" />
<Compile Include="src\AltBeacon\Logging\WarningDiagnosticsLogger.cs" />
<Compile Include="src\AltBeacon\Logging\WarningDiagnosticsLoggerFactory.cs" />
</ItemGroup>
<Import Project="$(MSBuildExtensionsPath32)\Microsoft\Portable\$(TargetFrameworkVersion)\Microsoft.Portable.CSharp.targets" />
<Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
<!-- To modify your build process, add your task inside one of the targets below and uncomment it.
Other similar extension points exist, see Microsoft.Common.targets.
<Target Name="BeforeBuild">
Expand Down
18 changes: 8 additions & 10 deletions AltBeacon.Library/src/AltBeacon/Beacon/Beacon.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ namespace AltBeacon.Beacon
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Text;
using AltBeacon.Logging;

/// <summary>
/// <para>
Expand Down Expand Up @@ -56,9 +57,9 @@ namespace AltBeacon.Beacon
public class Beacon
{
/// <summary>
/// Logger Tag
/// Current Class Logger Reference
/// </summary>
private const string Tag = "Beacon";
private static readonly ILogger Logger = LogManager.GetLogger("Beacon");

/// TODO DistanceCalculator
/*protected static DistanceCalculator sDistanceCalculator = null;*/
Expand Down Expand Up @@ -224,11 +225,10 @@ public double Distance
{
bestRssiAvailable = this.RunningAverageRssi.Value;
}
////TODO LogManager
////else
////{
//// LogManager.d(Tag, "Not using running average RSSI because it is null");
////}
else
{
Logger.Debug("Not using running average RSSI because it is null");
}

this.distance = CalculateDistance(this.TxPower, bestRssiAvailable);
}
Expand Down Expand Up @@ -283,7 +283,6 @@ public double Distance
/// </summary>
public int ServiceUuid { get; protected set; }


/// <summary>
/// Gets and sets the DistanceCalculator to use with this beacon.
/// </summary>
Expand Down Expand Up @@ -410,8 +409,7 @@ protected static double CalculateDistance(int txPower, double bestRssiAvailable)
////}
////else
////{
////TODO LogManager
////LogManager.e(Tag, "Distance calculator not set. Distance will bet set to -1");
////Logger.Error("Distance calculator not set. Distance will bet set to -1");
return -1.0;
////}
}
Expand Down
111 changes: 67 additions & 44 deletions AltBeacon.Library/src/AltBeacon/Beacon/BeaconParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ namespace AltBeacon.Beacon
using System.Text;
using System.Text.RegularExpressions;
using AltBeacon.Bluetooth;
using AltBeacon.Logging;

/// <summary>
/// <para>
Expand All @@ -52,9 +53,9 @@ public partial class BeaconParser
{
#region Static Fields
/// <summary>
/// Logger Tag
/// Current Class Logger Reference
/// </summary>
private const string Tag = "BeaconParser";
private static readonly ILogger Logger = LogManager.GetLogger("BeaconParser");

/// <summary>
/// Identifier Pattern
Expand Down Expand Up @@ -416,8 +417,7 @@ public BeaconParser SetBeaconLayout(string beaconLayout)

if (!found)
{
////TODO LogManager
////LogManager.d(Tag, "cannot parse term " + term);
Logger.Debug("cannot parse term " + term);
Copy link
Member

Choose a reason for hiding this comment

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

All of the debug logs inside the parser probably need to be wrapped in an if statement that checks to see if debug is enabled before constructing the string. It is really important to do that here because this code will run hundreds of times per second when lots of bluetooth devices are around.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, then the flag name "VerboseLoggingEnabled" must be changed to "BeaconParserLoggingEnabled", right ?

Copy link
Member

Choose a reason for hiding this comment

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

No, I don't think that is necessary. If you have VerboseLoggingEnabled set to false, lines like Logger.Debug("cannot parse term " + term); will do nothing anyway. All you are accomplishing by wrapping these in if statements like:

if (LogManager.VerboseLoggingEnabled) {
  Logger.Debug("cannot parse term " + term);
}

is making it so the string object construction and concatenation does not take place unnecessarily, which saves processing time. Even if you don't do this everywhere, doing it inside beacon parsing is important because it is gets executed in a tight loop.

Copy link
Member Author

Choose a reason for hiding this comment

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

Btw, I can write add LogManager.VerboseLoggingEnabled condition to every logger so that they wont log anything like it supposed to do and also if I use logger with params like Logger.Debug("cannot parse term {0}", term);, log string won't even get built (but of course in this case it will change nothing since there is only one param).

Also, this is a debug log not a verbose that is why I asked to change name of the flag since VerboseLoggingEnabled kind of misleading for disabling debug logs.

Copy link
Member

Choose a reason for hiding this comment

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

The flag could change to DebugLoggingEnabled, yes.

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, I will change the flag name. So what about the other thing that I told you above ? Does it sounds good or not even necessary ?

Also, what about the comments below ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have wrapped this log with condition and renamed VerboseLoggingEnabled to DebugLoggingEnabled

Copy link
Member

Choose a reason for hiding this comment

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

I don't think you have to "add LogManager.VerboseLoggingEnabled condition to every logger". It is only really necessary inside the parser where performance counts.

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, I have only wrapped one log which is proposed by you above.

throw new BeaconLayoutException("Cannot parse beacon layout term: " + term);
}
}
Expand Down Expand Up @@ -699,19 +699,16 @@ protected static string ByteArrayToFormattedString(byte[] byteBuffer, int startI
{
long number = 0L;

//// TODO LogManager
//// LogManager.d(Tag, "Byte array is size " + bytes.Length);
Logger.Debug("Byte array is size " + bytes.Length);
for (int i = 0; i < bytes.Length; i++)
{
//// TODO LogManager
//// LogManager.d(Tag, "index is " + i);
Logger.Debug("index is " + i);
long byteValue = (long)(bytes[bytes.Length - i - 1] & 0xff);
long positionValue = (long)Math.Pow(256.0, i * 1.0);
long calculatedValue = (long)(byteValue * positionValue);
//// TODO LogManager
//// LogManager.d(Tag, "calculatedValue for position " + i +
//// " with positionValue " + positionValue + " and byteValue " +
//// byteValue + " is " + calculatedValue);
Logger.Debug("calculatedValue for position " + i +
" with positionValue " + positionValue + " and byteValue " +
byteValue + " is " + calculatedValue);
number += calculatedValue;
}

Expand Down Expand Up @@ -795,18 +792,41 @@ protected static bool AreByteArraysMatch(byte[] array1, int offset1, byte[] arra
/// </returns>
protected Beacon FromScanData(byte[] scanData, int rssi, BluetoothDevice device, Beacon.Builder beaconBuilder)
{
int startByte = 2;
bool patternFound = false;
int matchingBeaconSize = this.matchingBeaconTypeCodeEndOffset.Value -
int maxByteForMatch = 5; // for manufacturer data-based beacons
byte[] serviceUuidBytes = null;

int matchingBeaconSize = this.matchingBeaconTypeCodeEndOffset.Value -
this.matchingBeaconTypeCodeStartOffset.Value + 1;
byte[] typeCodeBytes = LongToByteArray(this.MatchingBeaconTypeCode, matchingBeaconSize);

while (startByte <= 5)
if (this.ServiceUuid != null)
{
if (AreByteArraysMatch(scanData, startByte + this.matchingBeaconTypeCodeStartOffset.Value, typeCodeBytes, 0))
maxByteForMatch = 11; // for uuid-based beacons
int serviceUuidSize = this.ServiceUuidEndOffset - this.ServiceUuidStartOffset + 1;
serviceUuidBytes = LongToByteArray(this.ServiceUuid.Value, serviceUuidSize);
}

int startByte = 2;
bool patternFound = false;

while (startByte <= maxByteForMatch)
{
if (this.ServiceUuid == null)
{
patternFound = true;
break;
if (AreByteArraysMatch(scanData, startByte + this.MatchingBeaconTypeCodeStartOffset, typeCodeBytes, 0))
{
patternFound = true;
break;
}
}
else
{
if (AreByteArraysMatch(scanData, startByte + this.ServiceUuidStartOffset, serviceUuidBytes, 0) &&
AreByteArraysMatch(scanData, startByte + this.MatchingBeaconTypeCodeStartOffset, typeCodeBytes, 0))
{
patternFound = true;
break;
}
}

startByte++;
Expand All @@ -815,41 +835,44 @@ protected Beacon FromScanData(byte[] scanData, int rssi, BluetoothDevice device,
if (patternFound == false)
{
// This is not a beacon
// TODO LogManager
/*if (this.ServiceUuid == null)
if (this.ServiceUuid == null)
{
TODO LogManager
if (LogManager.isVerboseLoggingEnabled())
if (LogManager.VerboseLoggingEnabled)
{
LogManager.d(TAG, "This is not a matching Beacon advertisement. " +
"(Was expecting %s. The bytes I see are: %s",
byteArrayToString(typeCodeBytes), bytesToHex(scanData));
Logger.Debug(
"This is not a matching Beacon advertisement. " +
"(Was expecting {0}. The bytes I see are: {1}",
ByteArrayToString(typeCodeBytes),
BytesToHex(scanData));
}
}
else
{
if (LogManager.isVerboseLoggingEnabled())
if (LogManager.VerboseLoggingEnabled)
{
LogManager.d(TAG, "This is not a matching Beacon advertisement. " +
"(Was expecting %s and %s. The bytes I see are: %s",
byteArrayToString(serviceUuidBytes),
byteArrayToString(typeCodeBytes), bytesToHex(scanData));
Logger.Debug(
"This is not a matching Beacon advertisement. " +
"(Was expecting {0} and {1}. The bytes I see are: {2}",
ByteArrayToString(serviceUuidBytes),
ByteArrayToString(typeCodeBytes),
BytesToHex(scanData));
}
}*/
}

return null;
}
else
{
//// TODO LogManager
//// if (LogManager.isVerboseLoggingEnabled())
//// {
//// LogManager.d(TAG, "This is a recognized beacon advertisement -- %s seen",
//// byteArrayToString(typeCodeBytes));
//// }
////
//// TODO LogManager
//// LogManager.d(Tag, "This is a recognized beacon advertisement -- " +
//// getMatchingBeaconTypeCode().ToString("x4") + " seen");
if (LogManager.VerboseLoggingEnabled)
{
Logger.Debug(
"This is a recognized beacon advertisement -- {0} seen",
ByteArrayToString(typeCodeBytes));
}

Logger.Debug(
"This is a recognized beacon advertisement -- {0} seen",
this.MatchingBeaconTypeCode.ToString("x4"));
}

List<Identifier> identifiers = new List<Identifier>();
Expand All @@ -874,8 +897,8 @@ TODO LogManager
this.dataLittleEndianFlags[i]);

dataFields.Add(long.Parse(dataString));
//// TODO LogManager
//// LogManager.d(Tag, "parsing found data field " + i);

Logger.Debug("parsing found data field " + i);
//// TODO: error handling needed here on the parse
}

Expand Down
Loading