Feedback when trying to send package.

[ntilley905]

It would be useful for debugging purposes to get some kind of return from the send_magic_packet command to verify the call was successful.

[JordRoest]

Exactly. When I was testing this script to turn on my PC, it didn't work because I forgot a Windows setting. But I thought it was something in this script because I didn't get confirmation that the package was sent successfully. That was the moment i thought let's implement it.

[DavidPratt512]

Hey @JordRoest,

I think receiving optional feedback is a good idea. However, in this pull request, there is no functionality to enable the feedback from the command line.

Also, I have a few other thoughts:

1. Should print() be called directly from the function? If one was using this module in their code and they called send_magic_packet(), how could they programmatically check if the magic packet was sent? It's pretty hard to check if print() was called inside of a function.
2. How many times should the message be sent to stdout? You can easily imagine a use case where a sysadmin is trying to wake multiple machines with a single call to the script (as is intended). Their terminal would be flooded with the message Package has been sent.. If one packet failed to send, the message Package has not been sent. would appear in the sea of other messages. How easy would it be for the sysadmin to check which packet wasn't sent?
3. (nit-pick) If that message is to remain largely unchanged, should "Package" be changed to "Packet"?
4. The parameters on the send_magic_packet() function are given with type hints, so the feedback parameter should be given the type hint of bool.
5. In line 61, the meaning of 102 was unclear to me for a moment. Perhaps there should be a constant, say, EXPECTED_BYTES = 102 or: EXPECTED_BYTES = (12 + 12*16) // 2 to compare send to?
6. (nit-pick) Should the variable holding the number of bytes sent by the socket be called send or something along the lines of sent_bytes?
7. (nit-pick) Is it necessary to convert send and 102 to strings before doing the comparison?

Let me know your thoughts.

[remcohaszing]

Thank you for this contribution! I agree it’s useful to get some feedback, but I also share the concerns as pointed out by @DavidPratt512

I think there are two useful types of feedback to provide.

1. Logging

For logging, just create the following logger. It’s set to logging.ERROR by default, to keep it silent by default.

import logging

logger = logging.Logger('wakeonlan', logging.ERROR)

Then in the function, use:

if sent == 102:
    logger.info("Package has been sent.")
else:
    logger.warning("Package has not been sent.")

Optionally, add a CLI flag to enable logging.

2. Raising an error

For changing runtime code in case there was a failure sending the packet, it makes sense to raise a dedicated error:

class WakeOnLanError(Exception):
    """
    This error is raised when a magic packet could not be sent.

    """
    packet: bytes

    def __init__(self, message: str, packet: bytes):
      """
      Args:
          message: The error message to display
          packet: The magic packat that could not be sent.

      """
      super().__init__(message)
      self.packet = packet

Then this error could be raised after the warning has been logged.