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

Some features/fixes #6

Merged
merged 10 commits into from
Aug 3, 2018
Merged
1 change: 1 addition & 0 deletions CLR_DEV9/CLR_DEV9.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@
<DependentUpon>ConfigFormIncomingPorts.cs</DependentUpon>
</Compile>
<Compile Include="Config\ConfigHost.cs" />
<Compile Include="Config\ConfigLogging.cs" />
<Compile Include="Config\ConfigIncomingPort.cs" />
<Compile Include="Config\Test\Eth\ARPTest.cs" />
<Compile Include="Config\Test\Eth\DEV9_Test.cs" />
Expand Down
7 changes: 6 additions & 1 deletion CLR_DEV9/Config/ConfigFile.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using PSE;
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
Expand Down Expand Up @@ -31,6 +32,9 @@ class ConfigFile
[DataMember]
public int HddSize;

[DataMember]
public ConfigLogging EnableLogging;

[OnDeserializing]
void OnDeserializing(StreamingContext context)
{
Expand All @@ -47,6 +51,7 @@ private void Init()
HddEnable = false;
DirectConnectionSettings = new ConfigDirectIP();
SocketConnectionSettings = new ConfigSocketIP();
EnableLogging = new ConfigLogging();
Copy link
Owner

Choose a reason for hiding this comment

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

Do ConfigLogging's defaults apply correctly when loading an older config file (that lacks the entry)?

I remember having issues relating to that in the past.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If config file lacks the entry, it applies default values. (at least on my machine)
However, if you manually add required entries, it still applies default values until you let it recreate config file. (maybe I just did it in wrong order?)

Copy link
Owner

Choose a reason for hiding this comment

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

IIRC the entries in the file have to be in a specific order for settings to be read. I'm only concerned about updating an old file, and that seems to work.

Hosts = new HashSet<ConfigHost>();
Hosts.Add(new ConfigHost()
{
Expand Down Expand Up @@ -121,7 +126,7 @@ public static void LoadConf(string iniFolderPath, string iniFileName)
if (File.Exists(filePath))
{
DataContractSerializer ConfSerializer = new DataContractSerializer(typeof(ConfigFile));
FileStream Reader = new FileStream(filePath, FileMode.Open);;
FileStream Reader = new FileStream(filePath, FileMode.Open);
DEV9Header.config = (ConfigFile)ConfSerializer.ReadObject(Reader);
Reader.Close();
//Update from old config
Expand Down
16 changes: 16 additions & 0 deletions CLR_DEV9/Config/ConfigLogging.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
using System.Net;
using System.Runtime.Serialization;

namespace CLRDEV9.Config
{
[DataContract(Namespace = "http://schemas.datacontract.org/2004/07/CLRDEV9")]
class ConfigLogging
{
[DataMember]
public bool Error = true;
[DataMember]
public bool Verbose = true;
Copy link
Owner

Choose a reason for hiding this comment

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

Verbose should default to false to match release build behaviour.

[DataMember]
public bool Information = true;
}
}
5 changes: 3 additions & 2 deletions CLR_DEV9/DEV9.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using PSE;
using PSE.CLR_PSE_Callbacks;
using System;
using System.IO;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
Expand Down Expand Up @@ -89,10 +90,10 @@ public static Int32 Open(IntPtr winHandle)
ConfigFile.LoadConf(iniFolderPath, "CLR_DEV9.ini");
Log_Info("Config Loaded");

if (DEV9Header.config.Hdd.Contains("\\") || DEV9Header.config.Hdd.Contains("/"))
if (DEV9Header.config.Hdd.Contains(Path.DirectorySeparatorChar))
ret = dev9.Open(DEV9Header.config.Hdd);
else
ret = dev9.Open(iniFolderPath + "\\" + DEV9Header.config.Hdd);
ret = dev9.Open(iniFolderPath + Path.DirectorySeparatorChar + DEV9Header.config.Hdd);

if (ret == 0)
Log_Info("Open ok");
Expand Down
9 changes: 8 additions & 1 deletion CLR_DEV9/PSE/CLR_PSE_PluginLog.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using CLRDEV9;
Copy link
Owner

Choose a reason for hiding this comment

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

Remove the reference to CLRDEV9 as it's not needed.


namespace PSE
{
Expand Down Expand Up @@ -174,7 +175,13 @@ public static void Close()

public static void WriteLine(TraceEventType eType, int logSource, string str)
{
if (sources == null) return;
if (sources == null)
return;
if (DEV9Header.config != null)
if ((!DEV9Header.config.EnableLogging.Error && eType == TraceEventType.Error) ||
(!DEV9Header.config.EnableLogging.Verbose && eType == TraceEventType.Verbose) ||
(!DEV9Header.config.EnableLogging.Information && eType == TraceEventType.Information))
return;
Copy link
Owner

Choose a reason for hiding this comment

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

Currently, logging is configured with LogInit() in DEV9.cs

private static void LogInit()
Which currently does so based on build type (Debug or Release)

Note that LogInit() is called before the configuration file have been loaded, I'm not sure what the best option to resolve this is.

You can call SetStdOutLevel(), SetStdErrLevel() & SetFileLevel() to configure global restrictions for each output. Any log levels set in SetStdOutLevel() must be excluded inSetStdErrLevel() or else you will get duplicate messages.

Also you must allow TraceEventType.Critical to log to stderr and the log file, this level is only used here in the event of a fatal error.

public static void MsgBoxErrorTrapper(Exception e)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really sure how I should handle this, because as you said, LogInit() is called before the configuration file have been loaded.
When I made this, I was thinking only about how to stop these logs, I was not even thinking about build types, maybe because I can't find a way to select Release or Debug type in VS Code 0_o

Copy link
Owner

Choose a reason for hiding this comment

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

I haven't used VSCode much myself, but I've made build tasks that should allow you to create release builds. (You will need to open the solution folder instead of the project folder is vscode to use the tasks)

You could either move any calls to ConfigFile.LoadConf() before any LogInit() (by moving ConfigFile.LoadConf() from Open() to Init() and reordering the calls in Config()) or by splitting LogInit() into two functions with Initialise logging with reasonable defaults before a second function is called to set logging levels as desired.

Ideally I would like to be able to enable Verbose logging on a per source level as just one source can produce a large amount entries, although the need for verbose logs doesn't come up often.

if (sources.ContainsKey(logSource))
{
sources[logSource].TraceEvent(eType, logSource, str);
Expand Down
18 changes: 18 additions & 0 deletions CLR_DEV9_LINUX/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
cmake_minimum_required(VERSION 3.10)
project(clrdev9)

add_library(${PROJECT_NAME} SHARED
coreclrhost.h
DEV9.h
DEV9.cpp
PSE.h
PSE.cpp
)

set_target_properties(${PROJECT_NAME} PROPERTIES COMPILE_FLAGS "-m32" LINK_FLAGS "-m32")
target_link_libraries(${PROJECT_NAME}
m
rt
dl
pthread
)
8 changes: 6 additions & 2 deletions CLR_DEV9_LINUX/PSE.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <fcntl.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <pwd.h>
Copy link
Owner

Choose a reason for hiding this comment

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

Remove reference if this is no-longer required.


#include <set>
#include <vector>
Expand Down Expand Up @@ -303,8 +304,11 @@ void LoadCoreCLR(string pluginPath, string coreClrFolder)

if (coreClrFolder.length() == 0)
{
//coreClrFolder = "/home/air/git/ReadyBin.Release";
coreClrFolder = "/home/air/git/ReadyBin.Debug";
const char *homedir = getenv("HOME");
if ( homedir == NULL ) {
homedir = getpwuid(getuid())->pw_dir;
}
coreClrFolder = string(homedir) + string("/.config/PCSX2/coreclr");
Copy link
Owner

Choose a reason for hiding this comment

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

CLR_DEV9_CORE.dll is currently found bases on the path passed to DEV9setSettingsDir while this assumes it's always in the home directory (it can be configured otherwise) Both should probably be consistent to avoid any confusion if a non-default settings path is encountered.

}

string coreClrPath = coreClrFolder + "/" + "libcoreclr.so";
Expand Down