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

Implement missing miner functions on FreeBSD #2792

Merged
merged 2 commits into from
Nov 25, 2017

Conversation

vasild
Copy link
Contributor

@vasild vasild commented Nov 11, 2017

Implement missing miner functions on FreeBSD

cryptonote::miner::get_system_times(): Fetch the system's total and
idle time using sysctl kern.cp_time.

cryptonote::miner::get_process_time(): Use the same implementation as
Linux and OSX, the times(3) function conforms to POSIX.1 and is
available on FreeBSD.

cryptonote::miner::on_battery_power(): Try to fetch the battery status
using sysctl hw.acpi.acline. If that fails (if ACPI is not enabled on
the system), then try querying /dev/apm.

cryptonote::miner::get_system_times(): Fetch the system's total and
idle time using sysctl kern.cp_time.

cryptonote::miner::get_process_time(): Use the same implementation as
Linux and OSX, the times(3) function conforms to POSIX.1 and is
available on FreeBSD.

cryptonote::miner::on_battery_power(): Try to fetch the battery status
using sysctl hw.acpi.acline. If that fails (if ACPI is not enabled on
the system), then try querying /dev/apm.
Copy link
Collaborator

@moneromooo-monero moneromooo-monero left a comment

Choose a reason for hiding this comment

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

OK, though I'm not sure we want error messages if we fail to probe though, that might spam a bit on machines where it triggers ?

@vasild
Copy link
Contributor Author

vasild commented Nov 11, 2017

Errors from get_system_times() are not repeated and they are followed by another message clearly stating that mining is stopped: "get_system_times call failed, background mining will NOT work!".

Errors from on_battery_power() are indeed spamming the log every 10 seconds. I see that the Linux implementation of that function uses a mixture of LOG_PRINT_L0() and LOG_ERROR(). Should I leave it as is or convert the FreeBSD errors from LOG_ERROR() to LOG_PRINT_L0()?

@moneromooo-monero
Copy link
Collaborator

Level based messages (LOG_PRINT_Lx) are the original code, which was replaced with severity based messages (MERROR, MDEBUG, etc). The latter are prefered, and there is mapping from level based to severity based to avoid a huge pointless patch. LOG_ERROR probably points to MERROR, so is prefered over LOG_PRINT_L0 too in new code.

@vasild
Copy link
Contributor Author

vasild commented Nov 11, 2017

I see. So this patch is good to go or requires some adjustments?

@moneromooo-monero
Copy link
Collaborator

It's fine, thanks.

Copy link
Contributor

@fluffypony fluffypony left a comment

Choose a reason for hiding this comment

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

Reviewed

@fluffypony fluffypony merged commit 47c0948 into monero-project:master Nov 25, 2017
fluffypony added a commit that referenced this pull request Nov 25, 2017
47c0948 Implement missing miner functions on FreeBSD (Vasil Dimov)
fdb5bd1 Remove unused variables and fix typos in comments (Vasil Dimov)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants