fix: various concerns

- repackage logger from hm_pyhelper.utils to hm_pyhelper.logger
- LOGGER should be instantiated as uppercase by convention
- add tests
- add retry logic to get_region
- create await_spi_available helper
- fix override logic

README.md

@@ -63,27 +63,29 @@ Please note, DIY Hotspots do not earn HNT.
 | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- |
 | Pi Supply IoT LoRa Gateway HAT | RPi | DIY-PISLGH | 0.0 | 22 |   |   | Light | False | Any pi with 40 pin header |
 | RAK2287 | RPi | DIY-RAK2287 | 0.0 | 17 |   |   | Light | False | Any pi with 40 pin header |
-## utils
 
-### logger
+## logger
 
 ```python
-from hm_pyhelper.utils import logger
-logger = get_logger(__name__)
-logger.debug("message to log")
+from hm_pyhelper.logger import get_logger
+LOGGER = get_logger(__name__)
+LOGGER.debug("message to log")
 ```
 
 ## miner_param
 
-### get_region
-Return the region from envvar REGION_OVERRIDE or
-from the contents of /var/pktfwd/region
+### retry_get_region(region_override, region_filepath)
+Return the region from envvar region_override or
+from the contents of region_filepath
 
 ```python
-from hm_pyhelper.miner_param import get_region
-print(get_region())
+from hm_pyhelper.miner_param import retry_get_region
+print(retry_get_region("US915", "/invalid/path"))
+# US915
 
-> US9155
+# echo "EU868" > /var/pktfwd/region
+print(retry_get_region("", "/var/pktfwd/region"))
+# EU868
 ```
 
 ## Testing
@@ -95,3 +97,14 @@ pip install -r requirements.txt
 pip install -r test-requirements.txt
 PYTHONPATH=./ pytest
 ```
+
+## Referencing a branch for development
+It is sometimes convenient to use recent changes in hm-pyhelper before an official release.
+To do so, first double check that you've added any relevant dependencies to
+the `install_requires` section of `setup.py`. Then add the following lines to the
+project's Dockerfile.
+
+```Dockerfile
+RUN pip3 install setuptools wheel
+RUN pip3 install --target="$OUTPUTS_DIR" git+https://github.com/NebraLtd/hm-pyhelper@BRANCH_NAME
+``````

hm_pyhelper/exceptions.py

@@ -0,0 +1,6 @@
+class MalformedRegionException(Exception):
+    pass
+
+
+class SPIUnavailableException(Exception):
+    pass

hm_pyhelper/miner_param.py

@@ -3,9 +3,14 @@
 import logging
 import json
 from retry import retry
-from hm_pyhelper.utils.logger import get_logger
+from hm_pyhelper.logger import get_logger
+from hm_pyhelper.exceptions import MalformedRegionException, \
+    SPIUnavailableException
 
-logger = get_logger(__name__)
+LOGGER = get_logger(__name__)
+REGION_INVALID_SLEEP_SECONDS = 30
+REGION_FILE_MISSING_SLEEP_SECONDS = 60
+SPI_UNAVAILABLE_SLEEP_SECONDS = 60
 
 
 def log_stdout_stderr(sp_result):
@@ -179,34 +184,37 @@ def get_mac_address(path):
     return file.readline().strip().upper()
 
 
-REGION_OVERRIDE_KEY = 'REGION_OVERRIDE'
-REGION_FILEPATH = '/var/pktfwd/region'
-REGION_INVALID_SLEEP_SECONDS = 30
-REGION_FILE_MISSING_SLEEP_SECONDS = 60
-
-
-class MalformedRegionException(Exception):
-    pass
-
-
-@retry(MalformedRegionException, delay=REGION_INVALID_SLEEP_SECONDS, logger=logger) # noqa
-@retry(FileNotFoundError, delay=REGION_FILE_MISSING_SLEEP_SECONDS, logger=logger) # noqa
-def get_region():
+@retry(MalformedRegionException, delay=REGION_INVALID_SLEEP_SECONDS, logger=LOGGER) # noqa
+@retry(FileNotFoundError, delay=REGION_FILE_MISSING_SLEEP_SECONDS, logger=LOGGER) # noqa
+def retry_get_region(region_override, region_filepath):
     """
-    Return the region from the environment or parse file created by hm-miner.
+    Return the override if it exists, or parse file created by hm-miner.
+    region_override is the actual value,
+    not the name of the environment variable.
     Retry if region in file is malformed or not found.
     """
-    region = os.getenv(REGION_OVERRIDE_KEY, False)
-    if region:
-        return region
+    if region_override:
+        return region_override
 
-    logger.debug("No REGION_OVERRIDE defined, will retrieve from miner.")
-    with open(REGION_FILEPATH) as region_file:
+    LOGGER.debug("No region override set (value = %s), will retrieve from miner." % region_override)  # noqa: E501
+    with open(region_filepath) as region_file:
         region = region_file.read().rstrip('\n')
-        logger.debug("Region %s parsed from %s " % (region, REGION_FILEPATH))
+        LOGGER.debug("Region %s parsed from %s " % (region, region_filepath))
 
         is_region_valid = len(region) > 3
         if is_region_valid:
             return region
 
         raise MalformedRegionException("Region %s is invalid" % region)
+
+
+@retry(SPIUnavailableException, delay=SPI_UNAVAILABLE_SLEEP_SECONDS, logger=LOGGER) # noqa
+def await_spi_available(spi_bus):
+    """
+    Check that the SPI bus path exists, assuming it is in /dev/{spi_bus}
+    """
+    if os.path.exists('/dev/{}'.format(spi_bus)):
+        LOGGER.debug("SPI bus %s Configured Correctly" % spi_bus)
+        return True
+    else:
+        raise SPIUnavailableException("SPI bus %s not found!" % spi_bus)

hm_pyhelper/tests/test_logger.py

@@ -0,0 +1,29 @@
+from unittest import TestCase
+from hm_pyhelper.logger import get_logger, _log_format
+import re
+import logging
+
+
+class TestLogger(TestCase):
+    def test_get_logger(self):
+        logger = get_logger(__name__)
+
+        with self.assertLogs() as captured:
+            logger.debug("Hello world.")
+
+        # check that there is only one log message
+        self.assertEqual(len(captured.records), 1)
+        record = captured.records[0]
+        formatter = logging.Formatter(_log_format)
+        formatted_output = formatter.format(record)
+
+        # Do not check timestamp and filepath because those change
+        # based on the environment and run time
+        expected_partial_output_regex = re.escape(
+            " - [DEBUG] - hm_pyhelper.tests.test_logger -" +
+            " (test_logger.py).test_get_logger -- ")
+        expected_output_regex = ".*" + \
+            expected_partial_output_regex + ".*" + \
+            " - Hello world."
+        are_logs_correct = re.search(expected_output_regex, formatted_output)
+        self.assertTrue(are_logs_correct)

hm_pyhelper/tests/test_miner_param.py

@@ -1,8 +1,7 @@
+from hm_pyhelper.miner_param import retry_get_region, await_spi_available, \
+    provision_key, did_gateway_mfr_test_result_include_miner_key_pass
 import unittest
-from unittest.mock import patch
-from hm_pyhelper.miner_param import provision_key
-from hm_pyhelper.miner_param import \
-        did_gateway_mfr_test_result_include_miner_key_pass
+from unittest.mock import mock_open, patch
 
 ALL_PASS_GATEWAY_MFR_TESTS = [
     {
@@ -77,7 +76,6 @@ def stdout():
 
 
 class TestMinerParam(unittest.TestCase):
-
     @patch(
             'hm_pyhelper.miner_param.get_gateway_mfr_test_result',
             return_value={
@@ -161,3 +159,14 @@ def test_did_gateway_mfr_test_result_include_miner_key_pass(self):
                     get_gateway_mfr_test_result
                     )
         )
+
+    def test_get_region_from_override(self):
+        self.assertEqual(retry_get_region("foo", "bar/"), 'foo')
+
+    @patch("builtins.open", new_callable=mock_open, read_data="ZZ111\n")
+    def test_get_region_from_miner(self, _):
+        self.assertEqual(retry_get_region(False, "foo/"), 'ZZ111')  # noqa: E501
+
+    @patch("os.path.exists", return_value=True)
+    def test_is_spi_available(self, _):
+        self.assertTrue(await_spi_available("spiXY.Z"))

setup.py

@@ -3,7 +3,7 @@
 
 setup(
     name='hm_pyhelper',
-    version='0.8.4',
+    version='0.8.9',
     author="Nebra Ltd",
     author_email="[email protected]",
     description="Helium Python Helper",
@@ -12,7 +12,8 @@
     url="https://github.com/NebraLtd/hm-pyhelper",
     install_requires=[
         'requests>=2.26.0',
-        'jsonrpcclient==3.3.6'
+        'jsonrpcclient==3.3.6',
+        'retry==0.9.2'
     ],
     project_urls={
         "Bug Tracker": "https://github.com/NebraLtd/hm-pyhelper/issues",