Feat/date time #21
Feat/date time #21
Conversation
WalkthroughThe pull request introduces several changes across multiple GitHub Actions workflows and Python files. Key updates include renaming and restructuring the build tests workflow, adding synchronization jobs for stable releases, enhancing date and time display functionality in the Changes
Possibly related PRs
Suggested labels
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Outside diff range and nitpick comments (13)
.github/workflows/dev2master.yml (2)
8-9: Consider renaming the job for clarityThe current job name
build_and_publishdoesn't accurately reflect its action. Since this job is pushing changes from one branch to another, a more appropriate name might bepush_dev_to_master.Consider applying this change:
- build_and_publish: + push_dev_to_master:
11-20: LGTM: Checkout and push actions are correctly configuredThe checkout and push actions are set up correctly:
- The checkout action uses
fetch-depth: 0to ensure all history is available, which is necessary for pushing refs.- The 'dev' branch is explicitly specified as the ref to checkout.
- The push action is configured to use the GitHub token for authentication and targets the 'master' branch.
However, consider adding a step to verify the contents of the 'dev' branch before pushing to 'master'. This could help prevent unintended changes from being promoted.
Consider adding a verification step before the push action:
- name: Verify dev branch contents run: | git log -1 --pretty=format:"%h - %s" # Display last commit git diff --name-status master # Show changes compared to master - name: Push dev -> master uses: ad-m/github-push-action@master # ... (rest of the existing configuration)This step will display the last commit on the 'dev' branch and show the changes compared to 'master', allowing for a final check before pushing.
scripts/remove_alpha.py (1)
1-7: Enhance docstring and remove extra blank lineThe docstring provides a brief explanation, but it could be more informative. Consider expanding it to include:
- A more detailed description of what the script does.
- An example of how to use the script.
Also, remove the extra blank line (line 8) for better readability.
Here's a suggested improvement for the docstring:
""" Remove alpha designation from version file. This script updates the VERSION_ALPHA variable in the specified version file, setting its value to 0. It's typically used when merging to master to declare a stable version. Usage: python remove_alpha.py --version-file path/to/version.py """.github/workflows/publish_stable.yml (2)
Line range hint
64-72: Optimize thesync_devjob configurationThe
sync_devjob is a good addition to keep thedevbranch in sync withmasterafter a stable release. However, consider the following improvements:
- Remove the
fetch-depth: 0option as it's not necessary for this operation:- fetch-depth: 0 # otherwise, there would be errors pushing refs to the destination repository.
- Update the comment to reflect the actual purpose of this job:
+ # Sync the dev branch with the latest stable release
- Fix the indentation of the
usesline in the "Push master -> dev" step:- uses: ad-m/github-push-action@master + uses: ad-m/github-push-action@masterThese changes will improve the clarity and consistency of the workflow.
Line range hint
1-72: Overall workflow improvement with sync job additionThe addition of the
sync_devjob to this workflow is a positive change that enhances the release process. It ensures that thedevbranch is kept up-to-date with themasterbranch after a stable release, maintaining consistency across branches.However, to fully realize the benefits of this change:
- Ensure that the
publish_pypijob is using the correct branch (as mentioned in a previous comment).- Consider adding a step in the
sync_devjob to update the version in thedevbranch for the next development cycle. This could involve incrementing the patch version and adding an alpha suffix.These changes contribute to a more robust and maintainable release process.
scripts/update_version.py (1)
1-65: Overall assessment: Good implementation with room for improvement.The
update_version.pyscript provides a solid foundation for managing version updates according to semantic versioning principles. The main functions,read_versionandupdate_version, correctly implement the required logic.However, there are several areas where the script can be improved:
- Enhance error handling throughout the script.
- Improve code readability and maintainability.
- Implement more robust parsing and file handling methods.
- Use type hints and constants to make the code more self-documenting.
These improvements will make the script more resilient to errors, easier to maintain, and more aligned with Python best practices.
Consider adding unit tests for this script to ensure its reliability, especially given its critical role in version management.
🧰 Tools
🪛 Ruff
18-18: Ambiguous variable name:
l(E741)
19-19: Ambiguous variable name:
l(E741)
.github/workflows/publish_alpha.yml (2)
24-37: Consider using a more recent Python version.While Python 3.8 is stable, it's not the latest version. Consider using a more recent Python version (e.g., 3.9 or 3.10) to take advantage of newer features and improvements. This can be done by updating line 32:
- python-version: 3.8 + python-version: 3.10The rest of the setup looks good, with the necessary steps for checking out the code and installing build tools.
130-137: LGTM: Build and publish steps are well-implemented.The build and publish steps are correctly configured:
- Using
setup.pyfor building packages is standard.- The official PyPA action for publishing to PyPI is a good choice.
- The use of a secret for PyPI authentication is secure.
Minor suggestion: Consider using
python -m buildinstead ofpython setup.py sdist bdist_wheelfor building packages, as it's the more modern approach recommended by PyPA. This change would look like:- python setup.py sdist bdist_wheel + python -m buildMake sure to install the
buildpackage in the "Install Build Tools" step if you make this change.ovos_PHAL_plugin_mk1/arduino.py (2)
69-69: LGTM! Consider adding a payload to the "version" message.The addition of the "version" message emission is a good improvement. It allows other parts of the system to be notified about version information.
Consider adding a payload to the "version" message with the actual version information. This would make the message more useful for receivers. For example:
self.bus.emit(Message("version", {"version": system_version}))Where
system_versionwould be the actual version string or object.
146-146: LGTM! Consider using f-strings consistently.The addition of debug log statements improves the ability to troubleshoot issues with the
EnclosureWriter. Good job on enhancing the logging.For consistency, consider using f-strings for all the debug log statements. For example:
LOG.debug(f"Command being processed: {cmd}")This makes the code more readable and consistent with the f-string usage in the first debug statement.
Also applies to: 150-150, 155-155
ovos_PHAL_plugin_mk1/__init__.py (3)
518-522: Approved changes with a minor suggestion for consistency.The changes to the
on_displaymethod are good improvements:
- Proper initialization of
x_offsetandy_offsetas integers.- Addition of debug logging for better troubleshooting.
- Correct retrieval of the
clear_previousvalue from the message data.For consistency, consider renaming
clear_previoustoclear_prevto match the Arduino commandcP. This would make the code more aligned with the underlying protocol:-clear_previous = message.data.get("clear_previous", clear_previous) +clear_prev = message.data.get("clear_prev", clear_prev) -clear_previous = int(str(clear_previous) == "True") -clear_previous = "cP=" + str(clear_previous) + "," +clear_prev = int(str(clear_prev) == "True") +clear_prev = "cP=" + str(clear_prev) + ","Also applies to: 527-527
598-605: Approved new method with suggestions for improved robustness.The new
on_display_datemethod is a good addition for displaying the date. It correctly handles the display process by deactivating mouth events, showing the date, waiting, and then resetting.Consider the following improvements for better robustness:
- Add input validation to ensure the message contains the expected data:
def on_display_date(self, message=None): if not message or not message.data or "text" not in message.data: LOG.warning("Invalid message for displaying date") return self._deactivate_mouth_events() self.on_text(message) sleep(10) self.on_display_reset() self._activate_mouth_events()
- Consider making the display duration configurable, either through a parameter or a class attribute:
def on_display_date(self, message=None, display_duration=10): # ... (input validation) self.on_text(message) sleep(display_duration) # ... (rest of the method)These changes will make the method more flexible and less prone to errors from unexpected input.
Line range hint
1-655: Overall assessment: Good additions with room for improvementThe changes to
ovos_PHAL_plugin_mk1/__init__.pysuccessfully add new functionality for displaying date and time on the Mark 1 device. The new methods,on_display_dateandon_display_time, are well-integrated into the existing class structure and provide useful features.To further improve the code:
Consider creating a separate
DisplayManagerclass to handle the complexities of rendering different types of content (text, icons, time). This would help to reduce the complexity of theMycroftMark1class and improve separation of concerns.Implement comprehensive error handling and logging throughout the class, especially in the new methods, to make debugging easier and improve system robustness.
Add unit tests for the new functionality to ensure reliability and make future refactoring easier.
Review and update the class documentation to reflect the new capabilities for displaying date and time.
These improvements will enhance the maintainability and reliability of the code as the project continues to evolve.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
- .github/workflows/build_tests.yml (1 hunks)
- .github/workflows/dev2master.yml (1 hunks)
- .github/workflows/publish_alpha.yml (1 hunks)
- .github/workflows/publish_stable.yml (1 hunks)
- ovos_PHAL_plugin_mk1/init.py (3 hunks)
- ovos_PHAL_plugin_mk1/arduino.py (2 hunks)
- scripts/remove_alpha.py (1 hunks)
- scripts/update_version.py (1 hunks)
- version.py (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- version.py
🧰 Additional context used
🪛 actionlint
.github/workflows/publish_alpha.yml
55-55: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
55-55: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
55-55: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
55-55: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
112-112: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
🪛 Ruff
scripts/update_version.py
18-18: Ambiguous variable name:
l(E741)
19-19: Ambiguous variable name:
l(E741)
🔇 Additional comments (11)
.github/workflows/dev2master.yml (1)
1-6: LGTM: Workflow structure and purpose are clearThe workflow is well-structured for its purpose of manually pushing changes from the 'dev' branch to the 'master' branch. The use of
workflow_dispatchallows for controlled promotions, which is a good practice for managing releases.scripts/remove_alpha.py (1)
1-26: Overall assessment: Good foundation with room for improvementThe script successfully accomplishes its main goal of removing the alpha designation from a version file. It has a clear structure and uses appropriate Python modules for file manipulation and command-line argument parsing.
However, there are several areas where the script could be improved:
- Enhanced error handling throughout the script.
- Better user feedback on the success or failure of operations.
- More comprehensive documentation, including an improved docstring.
- Minor code style improvements, such as removing unnecessary blank lines.
Implementing the suggested changes will result in a more robust, user-friendly, and maintainable script. Despite these improvement opportunities, the current implementation is functional and serves its purpose well.
.github/workflows/build_tests.yml (2)
Line range hint
1-50: Approve changes with suggested improvementsThe update to the Python versions in the test matrix is a good step forward. The overall structure of the workflow remains solid and serves its purpose well. With the suggested improvements (adding Python 3.11 to the matrix, updating GitHub Actions versions, and reviewing ignored vulnerabilities), this workflow will be even more effective in ensuring the quality and security of your project.
Line range hint
19-20: Update GitHub Actions versions and review ignored vulnerabilities
- Consider updating the GitHub Actions to their latest versions:
- uses: actions/checkout@v3 + uses: actions/checkout@v4 - uses: actions/setup-python@v3 + uses: actions/setup-python@v4These updates might provide bug fixes and performance improvements.
The
gh-action-pip-auditaction might have a newer version available. Check if there's an update that could improve vulnerability detection.Review the list of ignored vulnerabilities:
- PYSEC-2023-228
- GHSA-9wx4-h78v-vm56
- GHSA-34jh-p97f-mpxf
- PYSEC-2022-43012
Ensure these are still relevant and cannot be addressed. If possible, try to resolve these vulnerabilities instead of ignoring them, as this would improve the overall security of your project.
To verify if these vulnerabilities are still relevant, you can run:
This script will help you determine if these vulnerabilities are still present in the latest versions of the affected packages. If they're not, you can remove them from the ignore list.
Also applies to: 41-50
.github/workflows/publish_stable.yml (1)
Line range hint
18-45: Verify the branch used in thepublish_pypijobThe
publish_pypijob is checking out thedevbranch, but this is a stable release workflow triggered by pushes to themasterbranch. This might lead to inconsistencies between the released version and the actual stable code.Consider changing the checkout step to use the
masterbranch instead:- ref: dev + ref: masterAlso, update the
create_releasestep:- commitish: dev + commitish: masterTo verify the current setup, run the following script:
scripts/update_version.py (1)
1-9: LGTM: Clear docstring and appropriate imports.The file header provides a concise explanation of the script's purpose and usage. The imports are relevant to the script's functionality.
.github/workflows/publish_alpha.yml (2)
1-23: LGTM: Workflow trigger and job setup are well-defined.The workflow trigger and job setup are appropriately configured for an alpha release process. The path exclusions are well-defined to prevent unnecessary builds, and using the latest Ubuntu for the job is a good practice.
94-104: LGTM: Version update and changelog generation are well-implemented.The use of a custom script for version updating provides flexibility, and the changelog generation using a well-known GitHub action is a good practice. The configuration for the changelog generation looks appropriate, limiting the number of issues to 50 for readability.
ovos_PHAL_plugin_mk1/arduino.py (2)
158-158: LGTM! Good addition for debugging.The new debug log statement in the
writemethod is a valuable addition. It will help in tracking the commands as they enter the queue, which can be crucial for debugging any issues related to command processing.
69-69: Overall, these changes improve system observability and communication.The modifications in this file enhance the system in two main ways:
- Improved logging: The additional debug statements in the
EnclosureWriterclass will make it easier to track the flow of commands and diagnose any issues.- Enhanced communication: The new "version" message emission in the
EnclosureReaderclass could be valuable for other parts of the system that need version information.These changes are well-aligned with good software engineering practices, improving the maintainability and debuggability of the code without altering its core functionality.
Also applies to: 146-146, 150-150, 155-155, 158-158
ovos_PHAL_plugin_mk1/__init__.py (1)
606-655:⚠️ Potential issueApproved new method with suggestions for improvements and potential issues.
The new
on_display_timemethod is a comprehensive implementation for displaying time on the device. It handles different time formats and centers the display, which is great.Consider the following improvements:
- Add input validation:
def on_display_time(self, message=None): if not message or not message.data or "text" not in message.data: LOG.warning("Invalid message for displaying time") return display_time = message.data["text"] if not (4 <= len(display_time) <= 5): LOG.warning(f"Invalid time format: {display_time}") return
Remove commented-out code to improve readability.
Break down the method into smaller, more focused methods:
def on_display_time(self, message=None): # ... (input validation) self._deactivate_mouth_events() self._clear_screen_for_time(display_time) self._display_centered_time(display_time) sleep(5) self.on_display_reset() self._activate_mouth_events() def _clear_screen_for_time(self, display_time): # ... (clear screen logic) def _display_centered_time(self, display_time): # ... (display time logic)There's a potential issue with the x-offset calculation:
xoffset = (32 - (4 * (len(display_time)) - 2)) / 2This assumes that all characters (including ':') are 4 pixels wide, which isn't true according to the code. Consider revising this calculation for more accurate centering.
To ensure that this method works correctly with various time formats, it would be beneficial to add unit tests. Can you confirm if there are existing tests for this functionality or if you'd like assistance in creating them?
.github/workflows/dev2master.yml
Outdated
| # This workflow will generate a distribution and upload it to PyPI | ||
|
|
||
| name: Push dev -> master | ||
| on: | ||
| workflow_dispatch: | ||
|
|
||
| jobs: | ||
| build_and_publish: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v3 | ||
| with: | ||
| fetch-depth: 0 # otherwise, there would be errors pushing refs to the destination repository. | ||
| ref: dev | ||
| - name: Push dev -> master | ||
| uses: ad-m/github-push-action@master | ||
|
|
||
| with: | ||
| github_token: ${{ secrets.GITHUB_TOKEN }} | ||
| branch: master |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider additional safeguards and improvements
While the workflow is functional for its purpose, consider the following enhancements:
- Error Handling: Add steps to handle potential errors and notify relevant team members.
- Pre-push Checks: Implement checks or tests to ensure the 'dev' branch is in a good state before pushing to 'master'.
- Concurrency Control: Add concurrency settings to prevent multiple workflow runs from interfering with each other.
Here's an example of how you might implement these improvements:
name: Push dev -> master
on:
workflow_dispatch:
# Prevent concurrent runs of this workflow
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true
jobs:
push_dev_to_master:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
with:
fetch-depth: 0
ref: dev
- name: Run pre-push checks
run: |
# Add your checks here, for example:
# - Run tests
# - Lint code
# - Check for merge conflicts with master
echo "Running pre-push checks..."
- name: Push dev -> master
uses: ad-m/github-push-action@master
with:
github_token: ${{ secrets.GITHUB_TOKEN }}
branch: master
- name: Notify on failure
if: failure()
uses: 8398a7/action-slack@v3
with:
status: ${{ job.status }}
text: 'Push to master failed!'
webhook_url: ${{ secrets.SLACK_WEBHOOK }}This example adds concurrency control, a placeholder for pre-push checks, and a notification step using Slack (you'll need to set up the Slack integration and add the webhook URL to your repository secrets).
| def update_alpha(version_file): | ||
| alpha_var_name = "VERSION_ALPHA" | ||
|
|
||
| for line in fileinput.input(version_file, inplace=True): | ||
| if line.startswith(alpha_var_name): | ||
| print(f"{alpha_var_name} = 0") | ||
| else: | ||
| print(line.rstrip('\n')) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling and add feedback in update_alpha function
The function correctly modifies the file, but it lacks error handling and doesn't provide feedback. Consider the following improvements:
- Add error handling for file operations.
- Provide feedback on whether the file was modified.
- Handle the case where VERSION_ALPHA is not found in the file.
Here's a suggested improvement:
def update_alpha(version_file):
alpha_var_name = "VERSION_ALPHA"
modified = False
try:
with fileinput.input(version_file, inplace=True) as file:
for line in file:
if line.startswith(alpha_var_name):
print(f"{alpha_var_name} = 0")
modified = True
else:
print(line.rstrip('\n'))
if not modified:
print(f"Warning: {alpha_var_name} not found in {version_file}")
else:
print(f"Successfully updated {version_file}")
except IOError as e:
print(f"Error updating {version_file}: {e}")
raise
return modifiedThis version adds error handling, provides feedback, and returns a boolean indicating whether the file was modified.
| if __name__ == "__main__": | ||
| parser = argparse.ArgumentParser( | ||
| description='Update the version based on the specified part (major, minor, build, alpha)') | ||
| parser.add_argument('--version-file', help='Path to the version.py file', required=True) | ||
|
|
||
| args = parser.parse_args() | ||
|
|
||
| update_alpha(abspath(args.version_file)) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance main block with error handling and user feedback
The main block correctly sets up argparse and calls the update_alpha function, but it lacks error handling and user feedback. Consider the following improvements:
- Add error handling for the
update_alphafunction call. - Provide clear output to the user about the success or failure of the operation.
- Use a more descriptive help message for the --version-file argument.
Here's a suggested improvement:
if __name__ == "__main__":
parser = argparse.ArgumentParser(
description='Remove alpha designation from the version file by setting VERSION_ALPHA to 0')
parser.add_argument('--version-file',
help='Path to the version.py file to be updated',
required=True)
args = parser.parse_args()
try:
version_file = abspath(args.version_file)
if update_alpha(version_file):
print(f"Successfully removed alpha designation in {version_file}")
else:
print(f"No changes were made to {version_file}")
except Exception as e:
print(f"Error: Failed to update {args.version_file}")
print(f"Reason: {str(e)}")
exit(1)This version adds error handling, provides clear user feedback, and uses a more descriptive help message for the --version-file argument.
.github/workflows/build_tests.yml
Outdated
| @@ -13,7 +13,7 @@ jobs: | |||
| strategy: | |||
| max-parallel: 2 | |||
| matrix: | |||
| python-version: [ 3.7, 3.8, 3.9, "3.10" ] | |||
| python-version: [ 3.8, 3.9, "3.10" ] | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider including Python 3.11 in the test matrix
The update to remove Python 3.7 from the test matrix is a good step, as it has reached its end-of-life. However, you might want to consider adding Python 3.11 to the matrix, as it's the current latest stable version.
- python-version: [ 3.8, 3.9, "3.10" ]
+ python-version: [ 3.8, 3.9, "3.10", "3.11" ]This would ensure that your package is tested against the most recent Python version, potentially catching any compatibility issues early.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| python-version: [ 3.8, 3.9, "3.10" ] | |
| python-version: [ 3.8, 3.9, "3.10", "3.11" ] |
| def read_version(version_file): | ||
| VERSION_MAJOR = 0 | ||
| VERSION_MINOR = 0 | ||
| VERSION_BUILD = 0 | ||
| VERSION_ALPHA = 0 | ||
|
|
||
| with open(version_file, 'r') as file: | ||
| content = file.read() | ||
| for l in content.split("\n"): | ||
| l = l.strip() | ||
| if l.startswith("VERSION_MAJOR"): | ||
| VERSION_MAJOR = int(l.split("=")[-1]) | ||
| elif l.startswith("VERSION_MINOR"): | ||
| VERSION_MINOR = int(l.split("=")[-1]) | ||
| elif l.startswith("VERSION_BUILD"): | ||
| VERSION_BUILD = int(l.split("=")[-1]) | ||
| elif l.startswith("VERSION_ALPHA"): | ||
| VERSION_ALPHA = int(l.split("=")[-1]) | ||
| return VERSION_MAJOR, VERSION_MINOR, VERSION_BUILD, VERSION_ALPHA |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve readability and robustness of read_version function.
The function logic is correct, but there are a few areas for improvement:
- Replace the single-letter variable
lwith a more descriptive name likeline. - Consider using a more robust parsing method, such as regular expressions, to extract version numbers.
- Add error handling for unexpected file formats or missing version components.
Here's a suggested refactoring:
import re
def read_version(version_file):
version_components = {
"VERSION_MAJOR": 0,
"VERSION_MINOR": 0,
"VERSION_BUILD": 0,
"VERSION_ALPHA": 0
}
pattern = re.compile(r'^(VERSION_[A-Z]+)\s*=\s*(\d+)')
with open(version_file, 'r') as file:
for line in file:
match = pattern.match(line.strip())
if match:
key, value = match.groups()
if key in version_components:
version_components[key] = int(value)
return tuple(version_components.values())This refactoring improves readability, uses a more robust parsing method, and maintains the original function signature.
🧰 Tools
🪛 Ruff
18-18: Ambiguous variable name:
l(E741)
19-19: Ambiguous variable name:
l(E741)
| def update_version(part, version_file): | ||
| VERSION_MAJOR, VERSION_MINOR, VERSION_BUILD, VERSION_ALPHA = read_version(version_file) | ||
|
|
||
| if part == 'major': | ||
| VERSION_MAJOR += 1 | ||
| VERSION_MINOR = 0 | ||
| VERSION_BUILD = 0 | ||
| VERSION_ALPHA = 1 | ||
| elif part == 'minor': | ||
| VERSION_MINOR += 1 | ||
| VERSION_BUILD = 0 | ||
| VERSION_ALPHA = 1 | ||
| elif part == 'build': | ||
| VERSION_BUILD += 1 | ||
| VERSION_ALPHA = 1 | ||
| elif part == 'alpha': | ||
| VERSION_ALPHA += 1 | ||
|
|
||
| with open(version_file, 'w') as file: | ||
| file.write(f"""# START_VERSION_BLOCK | ||
| VERSION_MAJOR = {VERSION_MAJOR} | ||
| VERSION_MINOR = {VERSION_MINOR} | ||
| VERSION_BUILD = {VERSION_BUILD} | ||
| VERSION_ALPHA = {VERSION_ALPHA} | ||
| # END_VERSION_BLOCK""") |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance robustness and maintainability of update_version function.
The function correctly implements semantic versioning logic, but there are opportunities for improvement:
- Add error handling for file operations.
- Consider preserving other content in the version file, if any.
- Use constants for version parts to improve maintainability.
Here's a suggested refactoring:
from enum import Enum
import re
class VersionPart(Enum):
MAJOR = 'major'
MINOR = 'minor'
BUILD = 'build'
ALPHA = 'alpha'
def update_version(part: VersionPart, version_file: str):
try:
VERSION_MAJOR, VERSION_MINOR, VERSION_BUILD, VERSION_ALPHA = read_version(version_file)
if part == VersionPart.MAJOR:
VERSION_MAJOR += 1
VERSION_MINOR = VERSION_BUILD = 0
VERSION_ALPHA = 1
elif part == VersionPart.MINOR:
VERSION_MINOR += 1
VERSION_BUILD = 0
VERSION_ALPHA = 1
elif part == VersionPart.BUILD:
VERSION_BUILD += 1
VERSION_ALPHA = 1
elif part == VersionPart.ALPHA:
VERSION_ALPHA += 1
version_block = f"""# START_VERSION_BLOCK
VERSION_MAJOR = {VERSION_MAJOR}
VERSION_MINOR = {VERSION_MINOR}
VERSION_BUILD = {VERSION_BUILD}
VERSION_ALPHA = {VERSION_ALPHA}
# END_VERSION_BLOCK"""
with open(version_file, 'r+') as file:
content = file.read()
file.seek(0)
file.write(re.sub(r'# START_VERSION_BLOCK.*# END_VERSION_BLOCK', version_block, content, flags=re.DOTALL))
file.truncate()
except IOError as e:
print(f"Error updating version file: {e}")
raiseThis refactoring adds error handling, preserves other file content, and uses an enum for version parts to improve maintainability.
| if __name__ == "__main__": | ||
| parser = argparse.ArgumentParser(description='Update the version based on the specified part (major, minor, build, alpha)') | ||
| parser.add_argument('part', help='Part of the version to update (major, minor, build, alpha)') | ||
| parser.add_argument('--version-file', help='Path to the version.py file', required=True) | ||
|
|
||
| args = parser.parse_args() | ||
|
|
||
| update_version(args.part, abspath(args.version_file)) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling to the main execution block.
The argument parsing is implemented correctly, but there's room for improvement:
- Add error handling for the
update_versionfunction call. - Validate the
partargument against allowed values.
Here's a suggested improvement:
if __name__ == "__main__":
parser = argparse.ArgumentParser(description='Update the version based on the specified part (major, minor, build, alpha)')
parser.add_argument('part', choices=['major', 'minor', 'build', 'alpha'], help='Part of the version to update')
parser.add_argument('--version-file', help='Path to the version.py file', required=True)
args = parser.parse_args()
try:
update_version(VersionPart(args.part), abspath(args.version_file))
print(f"Successfully updated {args.part} version in {args.version_file}")
except Exception as e:
print(f"Error updating version: {e}")
exit(1)This improvement adds error handling and validates the part argument against allowed values.
.github/workflows/publish_alpha.yml
Outdated
| - name: Debug GitHub Labels | ||
| run: | | ||
| echo "Labels in Pull Request:" | ||
| echo "${{ toJson(github.event.pull_request.labels) }}" | ||
|
|
||
| # Convert the labels array into text using jq | ||
| LABELS=$(echo '${{ toJson(github.event.pull_request.labels) }}' | jq -r '.[].name') | ||
|
|
||
| # Handle the case where there are no labels | ||
| if [ -z "$LABELS" ]; then | ||
| echo "No labels found on the pull request." | ||
| else | ||
| echo "Labels: $LABELS" | ||
| fi | ||
|
|
||
| - name: Determine version bump | ||
| id: version_bump | ||
| run: | | ||
| # Convert the labels array into text using jq | ||
| LABELS=$(echo '${{ toJson(github.event.pull_request.labels) }}' | jq -r '.[].name') | ||
|
|
||
| # Handle the case where there are no labels | ||
| if [ -z "$LABELS" ]; then | ||
| echo "No labels found on the pull request." | ||
| LABELS="" | ||
| fi | ||
|
|
||
| echo "Labels: $LABELS" | ||
|
|
||
| MAJOR=0 | ||
| MINOR=0 | ||
| BUILD=0 | ||
|
|
||
| # Loop over the labels and determine the version bump | ||
| for label in $LABELS; do | ||
| echo "Processing label: $label" | ||
| if [ "$label" == "breaking" ]; then | ||
| MAJOR=1 | ||
| elif [ "$label" == "feature" ]; then | ||
| MINOR=1 | ||
| elif [ "$label" == "fix" ]; then | ||
| BUILD=1 | ||
| fi | ||
| done | ||
|
|
||
| # Set the output based on the labels found | ||
| if [ $MAJOR -eq 1 ]; then | ||
| echo "::set-output name=part::major" | ||
| elif [ $MINOR -eq 1 ]; then | ||
| echo "::set-output name=part::minor" | ||
| elif [ $BUILD -eq 1 ]; then | ||
| echo "::set-output name=part::build" | ||
| else | ||
| echo "::set-output name=part::alpha" | ||
| fi | ||
|
|
There was a problem hiding this comment.
Update deprecated set-output command and improve label handling.
- The
set-outputcommand used in this section is deprecated. Replace it with the new syntax as follows:
- echo "::set-output name=part::major"
+ echo "part=major" >> $GITHUB_OUTPUTMake similar changes for other set-output commands in this section.
- Consider simplifying the label handling logic by using a case statement instead of multiple if-else conditions. This can make the code more readable and easier to maintain:
case "$label" in
"breaking")
echo "part=major" >> $GITHUB_OUTPUT
;;
"feature")
echo "part=minor" >> $GITHUB_OUTPUT
;;
"fix")
echo "part=build" >> $GITHUB_OUTPUT
;;
*)
echo "part=alpha" >> $GITHUB_OUTPUT
;;
esacThis change will make the script more efficient and easier to extend in the future.
🧰 Tools
🪛 actionlint
55-55: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
55-55: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
55-55: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
55-55: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
.github/workflows/publish_alpha.yml
Outdated
| - name: Commit to dev | ||
| uses: stefanzweifel/git-auto-commit-action@v4 | ||
| with: | ||
| commit_message: Increment Version | ||
| branch: dev | ||
|
|
||
| - name: version | ||
| run: echo "::set-output name=version::$(python setup.py --version)" | ||
| id: version | ||
|
|
||
| - name: Create Release | ||
| id: create_release | ||
| uses: actions/create-release@v1 | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # This token is provided by Actions, you do not need to create your own token | ||
| with: | ||
| tag_name: V${{ steps.version.outputs.version }} | ||
| release_name: Release ${{ steps.version.outputs.version }} | ||
| body: | | ||
| Changes in this Release | ||
| ${{ steps.changelog.outputs.changelog }} | ||
| draft: false | ||
| prerelease: true | ||
| commitish: dev | ||
|
|
There was a problem hiding this comment.
Update deprecated set-output command; rest LGTM.
The commit, release creation, and version retrieval steps are well-implemented. However, there's one issue to address:
- Replace the deprecated
set-outputcommand on line 112 with the new syntax:
- run: echo "::set-output name=version::$(python setup.py --version)"
+ run: echo "version=$(python setup.py --version)" >> $GITHUB_OUTPUTThe rest of this section looks good. The release is correctly marked as a pre-release, which is appropriate for alpha builds.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Commit to dev | |
| uses: stefanzweifel/git-auto-commit-action@v4 | |
| with: | |
| commit_message: Increment Version | |
| branch: dev | |
| - name: version | |
| run: echo "::set-output name=version::$(python setup.py --version)" | |
| id: version | |
| - name: Create Release | |
| id: create_release | |
| uses: actions/create-release@v1 | |
| env: | |
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # This token is provided by Actions, you do not need to create your own token | |
| with: | |
| tag_name: V${{ steps.version.outputs.version }} | |
| release_name: Release ${{ steps.version.outputs.version }} | |
| body: | | |
| Changes in this Release | |
| ${{ steps.changelog.outputs.changelog }} | |
| draft: false | |
| prerelease: true | |
| commitish: dev | |
| - name: Commit to dev | |
| uses: stefanzweifel/git-auto-commit-action@v4 | |
| with: | |
| commit_message: Increment Version | |
| branch: dev | |
| - name: version | |
| run: echo "version=$(python setup.py --version)" >> $GITHUB_OUTPUT | |
| id: version | |
| - name: Create Release | |
| id: create_release | |
| uses: actions/create-release@v1 | |
| env: | |
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # This token is provided by Actions, you do not need to create your own token | |
| with: | |
| tag_name: V${{ steps.version.outputs.version }} | |
| release_name: Release ${{ steps.version.outputs.version }} | |
| body: | | |
| Changes in this Release | |
| ${{ steps.changelog.outputs.changelog }} | |
| draft: false | |
| prerelease: true | |
| commitish: dev |
🧰 Tools
🪛 actionlint
112-112: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
|
this is not using the correct automations |
ovos_PHAL_plugin_mk1/__init__.py
Outdated
| '9': 'EIMBEBMHAA', | ||
| } | ||
|
|
||
| if message and message.data: |
There was a problem hiding this comment.
this should be impossible to reach without a message object, if its triggered via bus the message is always there
ovos_PHAL_plugin_mk1/arduino.py
Outdated
| @@ -66,6 +66,7 @@ def process(self, data): | |||
| # This happens in response to the "system.version" message | |||
| # sent during the construction of Enclosure() | |||
| self.bus.emit(Message("enclosure.started")) | |||
| self.bus.emit(Message("version")) | |||
There was a problem hiding this comment.
why was this added? if we are introducing a new message make it mk1 specific, version is too generic, also no message.data?
There was a problem hiding this comment.
It was for testing and I forgot to take it back out. But, system.version does not work, only version works, and only if the newest firmware is loaded to the arduino. Otherwise it just returns an echo. Eventually that is how I will do validation and auto upgrade of firmware.
There was a problem hiding this comment.
but that would be writen to the serial, not in messagebus?
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (5)
.github/workflows/license_tests.yml (1)
11-11: Remove trailing spaces.There are trailing spaces at the end of line 11. While this doesn't affect functionality, it's a good practice to maintain consistent formatting.
Apply this diff to remove the trailing spaces:
- +🧰 Tools
🪛 yamllint
[error] 11-11: trailing spaces
(trailing-spaces)
.github/workflows/build_tests.yml (2)
1-4: Workflow simplification looks good, but consider expanding triggers.The workflow has been renamed to "Unit Tests" which is more precise. The triggers have been simplified, which is generally good. However, you might want to consider adding specific branch conditions to avoid unnecessary runs.
Consider adding branch conditions to optimize workflow runs:
on: push: branches: [ main, develop ] pull_request: branches: [ main, develop ] workflow_dispatch:This ensures the workflow runs on important branches and PRs targeting them, while still allowing manual triggers.
11-11: Remove trailing whitespace.There's a trailing whitespace on line 11.
Apply this diff to remove the trailing whitespace:
- +🧰 Tools
🪛 yamllint
[error] 11-11: trailing spaces
(trailing-spaces)
.github/workflows/release_workflow.yml (1)
21-35: LGTM: New notification job for merged PRs.The new
notifyjob is a great addition to keep the team informed about merged PRs. It's correctly set up to run only when a PR is merged and after thepublish_alphajob succeeds.A minor suggestion for improvement:
Consider adding more details to the notification message, such as the PR title or a brief description of the changes. This would provide more context in the Matrix channel.
Example:
message: | New PR merged in ${{ github.event.repository.name }}! Title: ${{ github.event.pull_request.title }} URL: ${{ github.event.pull_request.html_url }}🧰 Tools
🪛 actionlint
26-26: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
ovos_PHAL_plugin_mk1/arduino.py (1)
Line range hint
1-160: Consider adding similar logging to EnclosureReaderThe additions to
EnclosureWriterimprove debugging capabilities. For consistency and to further enhance debugging, consider adding similar debug logging to key points in theEnclosureReaderclass, such as when data is read or when specific commands are processed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- .github/workflows/build_tests.yml (1 hunks)
- .github/workflows/license_tests.yml (1 hunks)
- .github/workflows/release_workflow.yml (2 hunks)
- ovos_PHAL_plugin_mk1/init.py (3 hunks)
- ovos_PHAL_plugin_mk1/arduino.py (1 hunks)
🧰 Additional context used
🪛 yamllint
.github/workflows/build_tests.yml
[error] 11-11: trailing spaces
(trailing-spaces)
.github/workflows/license_tests.yml
[error] 11-11: trailing spaces
(trailing-spaces)
🪛 actionlint
.github/workflows/release_workflow.yml
26-26: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (10)
.github/workflows/license_tests.yml (2)
1-7: Improved workflow trigger configuration.The changes to the workflow triggers enhance flexibility and focus:
- Adding
workflow_dispatchallows manual triggering, which is useful for on-demand runs.- Modifying the
pull_requestevent to target only themasterbranch aligns the license checks with the main development line.These updates provide better control over when and how the license tests are executed.
9-11: Streamlined job definition using external workflow.Replacing the entire
license_testsjob with a reference to an external workflow (neongeckocom/.github/.github/workflows/license_tests.yml@master) is a significant improvement:
- It centralizes the license testing logic, promoting consistency across repositories.
- It simplifies maintenance by allowing updates to be made in a single location.
- It reduces the risk of divergence in license testing procedures across projects.
This change aligns with best practices for CI/CD management in a multi-repository environment.
🧰 Tools
🪛 yamllint
[error] 11-11: trailing spaces
(trailing-spaces)
.github/workflows/build_tests.yml (1)
7-10: Verify the impact of using a single Python version.The job now uses an external workflow and specifies only Python 3.8. This simplifies the configuration, but it might limit the test coverage.
Please confirm if testing only on Python 3.8 is sufficient for your project's requirements. If not, consider specifying multiple versions:
with: python_version: '["3.8", "3.9", "3.10", "3.11"]'Would you like me to create an issue to discuss the Python version coverage in your CI pipeline?
.github/workflows/release_workflow.yml (2)
10-10: LGTM: Conditional execution for merged PRs.The added condition
if: github.event.pull_request.merged == trueensures that thepublish_alphajob only runs when a pull request is actually merged. This is a good practice to prevent unnecessary alpha releases for unmerged pull requests.
38-38: LGTM: Correct dependency added to publish_pypi job.The addition of
needs: publish_alphaensures that thepublish_pypijob runs only after the successful completion of thepublish_alphajob. This maintains the correct order of operations in the release process.ovos_PHAL_plugin_mk1/arduino.py (3)
145-145: LGTM: Useful debug logging addedThe addition of this debug log statement is beneficial. It provides visibility into the thread's state when the
flushmethod is called, which can be valuable for debugging purposes.
149-149: LGTM: Helpful command logging addedThis debug log statement is a good addition. It allows tracking of each command as it's processed in the
flushmethod, which can be invaluable for debugging and understanding the flow of commands.
157-157: LGTM: Effective command queue loggingThe addition of this debug log statement in the
writemethod is beneficial. It provides visibility into which commands are being added to the queue, which can be crucial for debugging and understanding the sequence of operations.ovos_PHAL_plugin_mk1/__init__.py (2)
88-90: Event handlers for date and time display registered successfullyThe event handlers for
"ovos.mk1.display_date"and"ovos.mk1.display_time"are correctly registered to their respective methods, enabling the new date and time display functionality.
599-605:on_display_datemethod implementation is appropriateThe
on_display_datemethod effectively handles displaying the date, pauses for 10 seconds, and then resets the display, ensuring a smooth user experience.
| py_build_tests: | ||
| uses: neongeckocom/.github/.github/workflows/python_build_tests.yml@master | ||
| with: | ||
| python_version: "3.8" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reconsider Python version coverage.
A previous review suggested including Python 3.11 in the test matrix. While the current implementation has moved away from a matrix approach, it's worth reconsidering the Python versions you're testing against.
If your project supports multiple Python versions, consider updating the workflow to test against them:
with:
python_version: '["3.8", "3.9", "3.10", "3.11"]'This ensures compatibility across supported Python versions, including the latest stable release (3.11).
| needs: publish_alpha | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v2 |
There was a problem hiding this comment.
Update actions/checkout action version.
The static analysis tool has flagged that the version of the actions/checkout action (v2) is outdated. It's recommended to update to the latest version to ensure compatibility and access to the latest features and bug fixes.
Please update the action version as follows:
- - uses: actions/checkout@v2
+ - uses: actions/checkout@v3This change will ensure you're using the most up-to-date version of the checkout action.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - uses: actions/checkout@v2 | |
| - uses: actions/checkout@v3 |
🧰 Tools
🪛 actionlint
26-26: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
| def on_display_time(self, message=None): | ||
| code_dict = { | ||
| ':': 'CIICAA', | ||
| '0': 'EIMHEEMHAA', | ||
| '1': 'EIIEMHAEAA', | ||
| '2': 'EIEHEFMFAA', | ||
| '3': 'EIEFEFMHAA', | ||
| '4': 'EIMBABMHAA', | ||
| '5': 'EIMFEFEHAA', | ||
| '6': 'EIMHEFEHAA', | ||
| '7': 'EIEAEAMHAA', | ||
| '8': 'EIMHEFMHAA', | ||
| '9': 'EIMBEBMHAA', | ||
| } | ||
|
|
||
| self._deactivate_mouth_events() | ||
| display_time = message.data.get("text") | ||
| # clear screen (draw two blank sections, numbers cover rest) | ||
| if len(display_time) == 4: | ||
| # for 4-character times, 9x8 blank | ||
| self.on_display(Message("", data={"img_code": "JIAAAAAAAAAAAAAAAAAA", "clear_previous": False})) | ||
| # self.enclosure.mouth_display(img_code="JIAAAAAAAAAAAAAAAAAA", | ||
| # refresh=False) | ||
| self.on_display(Message("", data={"img_code": "JIAAAAAAAAAAAAAAAAAA", "xOffset": 22, "clear_previous": False})) | ||
| # self.enclosure.mouth_display(img_code="JIAAAAAAAAAAAAAAAAAA", | ||
| # x=22, refresh=False) | ||
| else: | ||
| # for 5-character times, 7x8 blank | ||
| self.on_display(Message("", data={"img_code": "HIAAAAAAAAAAAAAA", "clear_previous": False})) | ||
| # self.enclosure.mouth_display(img_code="HIAAAAAAAAAAAAAA", | ||
| # refresh=False) | ||
| self.on_display(Message("", data={"img_code": "HIAAAAAAAAAAAAAA", "xOffset": 24, "clear_previous": False})) | ||
| # self.enclosure.mouth_display(img_code="HIAAAAAAAAAAAAAA", | ||
| # x=24, refresh=False) | ||
|
|
||
| # draw the time, centered on display | ||
| xoffset = (32 - (4 * (len(display_time)) - 2)) / 2 | ||
| for c in display_time: | ||
| if c in code_dict: | ||
| self.on_display(Message("", data={"img_code": code_dict[c], "xOffset": xoffset, "clear_previous": False})) | ||
| if c == ":": | ||
| xoffset += 2 # colon is 1 pixels + a space | ||
| else: | ||
| xoffset += 4 # digits are 3 pixels + a space | ||
|
|
||
| self.on_display(Message("", data={"img_code": "CIAAAA", "xOffset": 29, "clear_previous": False})) | ||
| sleep(5) | ||
| self.on_display_reset() | ||
| self._activate_mouth_events() |
There was a problem hiding this comment.
Ensure xoffset is an integer to prevent potential type errors
In the on_display_time method, xoffset is calculated using division, which results in a float value. Since the xOffset parameter should be an integer, this could cause issues when positioning the display elements. Please use integer division to ensure xoffset is an integer.
Apply this diff to fix the issue:
- xoffset = (32 - (4 * (len(display_time)) - 2)) / 2
+ xoffset = (32 - (4 * (len(display_time)) - 2)) // 2📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def on_display_time(self, message=None): | |
| code_dict = { | |
| ':': 'CIICAA', | |
| '0': 'EIMHEEMHAA', | |
| '1': 'EIIEMHAEAA', | |
| '2': 'EIEHEFMFAA', | |
| '3': 'EIEFEFMHAA', | |
| '4': 'EIMBABMHAA', | |
| '5': 'EIMFEFEHAA', | |
| '6': 'EIMHEFEHAA', | |
| '7': 'EIEAEAMHAA', | |
| '8': 'EIMHEFMHAA', | |
| '9': 'EIMBEBMHAA', | |
| } | |
| self._deactivate_mouth_events() | |
| display_time = message.data.get("text") | |
| # clear screen (draw two blank sections, numbers cover rest) | |
| if len(display_time) == 4: | |
| # for 4-character times, 9x8 blank | |
| self.on_display(Message("", data={"img_code": "JIAAAAAAAAAAAAAAAAAA", "clear_previous": False})) | |
| # self.enclosure.mouth_display(img_code="JIAAAAAAAAAAAAAAAAAA", | |
| # refresh=False) | |
| self.on_display(Message("", data={"img_code": "JIAAAAAAAAAAAAAAAAAA", "xOffset": 22, "clear_previous": False})) | |
| # self.enclosure.mouth_display(img_code="JIAAAAAAAAAAAAAAAAAA", | |
| # x=22, refresh=False) | |
| else: | |
| # for 5-character times, 7x8 blank | |
| self.on_display(Message("", data={"img_code": "HIAAAAAAAAAAAAAA", "clear_previous": False})) | |
| # self.enclosure.mouth_display(img_code="HIAAAAAAAAAAAAAA", | |
| # refresh=False) | |
| self.on_display(Message("", data={"img_code": "HIAAAAAAAAAAAAAA", "xOffset": 24, "clear_previous": False})) | |
| # self.enclosure.mouth_display(img_code="HIAAAAAAAAAAAAAA", | |
| # x=24, refresh=False) | |
| # draw the time, centered on display | |
| xoffset = (32 - (4 * (len(display_time)) - 2)) / 2 | |
| for c in display_time: | |
| if c in code_dict: | |
| self.on_display(Message("", data={"img_code": code_dict[c], "xOffset": xoffset, "clear_previous": False})) | |
| if c == ":": | |
| xoffset += 2 # colon is 1 pixels + a space | |
| else: | |
| xoffset += 4 # digits are 3 pixels + a space | |
| self.on_display(Message("", data={"img_code": "CIAAAA", "xOffset": 29, "clear_previous": False})) | |
| sleep(5) | |
| self.on_display_reset() | |
| self._activate_mouth_events() | |
| def on_display_time(self, message=None): | |
| code_dict = { | |
| ':': 'CIICAA', | |
| '0': 'EIMHEEMHAA', | |
| '1': 'EIIEMHAEAA', | |
| '2': 'EIEHEFMFAA', | |
| '3': 'EIEFEFMHAA', | |
| '4': 'EIMBABMHAA', | |
| '5': 'EIMFEFEHAA', | |
| '6': 'EIMHEFEHAA', | |
| '7': 'EIEAEAMHAA', | |
| '8': 'EIMHEFMHAA', | |
| '9': 'EIMBEBMHAA', | |
| } | |
| self._deactivate_mouth_events() | |
| display_time = message.data.get("text") | |
| # clear screen (draw two blank sections, numbers cover rest) | |
| if len(display_time) == 4: | |
| # for 4-character times, 9x8 blank | |
| self.on_display(Message("", data={"img_code": "JIAAAAAAAAAAAAAAAAAA", "clear_previous": False})) | |
| # self.enclosure.mouth_display(img_code="JIAAAAAAAAAAAAAAAAAA", | |
| # refresh=False) | |
| self.on_display(Message("", data={"img_code": "JIAAAAAAAAAAAAAAAAAA", "xOffset": 22, "clear_previous": False})) | |
| # self.enclosure.mouth_display(img_code="JIAAAAAAAAAAAAAAAAAA", | |
| # x=22, refresh=False) | |
| else: | |
| # for 5-character times, 7x8 blank | |
| self.on_display(Message("", data={"img_code": "HIAAAAAAAAAAAAAA", "clear_previous": False})) | |
| # self.enclosure.mouth_display(img_code="HIAAAAAAAAAAAAAA", | |
| # refresh=False) | |
| self.on_display(Message("", data={"img_code": "HIAAAAAAAAAAAAAA", "xOffset": 24, "clear_previous": False})) | |
| # self.enclosure.mouth_display(img_code="HIAAAAAAAAAAAAAA", | |
| # x=24, refresh=False) | |
| # draw the time, centered on display | |
| xoffset = (32 - (4 * (len(display_time)) - 2)) // 2 | |
| for c in display_time: | |
| if c in code_dict: | |
| self.on_display(Message("", data={"img_code": code_dict[c], "xOffset": xoffset, "clear_previous": False})) | |
| if c == ":": | |
| xoffset += 2 # colon is 1 pixels + a space | |
| else: | |
| xoffset += 4 # digits are 3 pixels + a space | |
| self.on_display(Message("", data={"img_code": "CIAAAA", "xOffset": 29, "clear_previous": False})) | |
| sleep(5) | |
| self.on_display_reset() | |
| self._activate_mouth_events() |
|
closing in favor of #15 |
Companion to OpenVoiceOS/ovos-skill-date-time#53
Summary by CodeRabbit
Release Notes
New Features
Workflow Enhancements
Bug Fixes
Documentation