Accelerometer code and Integrating sysID code #7
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
prasadtalasila
left a comment
prasadtalasila
left a comment
There was a problem hiding this comment.
@8ohamed Thanks for the PR. Please see the suggested changes.
| else: | ||
| avg = {} | ||
| for ax in axis: | ||
| total = sum(sample.get("accel", {}).get(ax, 0) for sample in collected_samples) |
Contributor
There was a problem hiding this comment.
why is it being averaged? Can you phrase the question properly and send a reply to our email conversation with Swarup?
src/cp-sens/methods/sysID.py
Outdated
| @@ -0,0 +1,40 @@ | |||
| import numpy as np #type: ignore | |||
| from pyoma2.setup.single import SingleSetup | |||
| from pyoma2.algorithms.ssi import SSIcov | |||
Contributor
There was a problem hiding this comment.
The idea is to wrap some of pyoma code the one provided by Swarup. Please see issue #7
Contributor
prasadtalasila
left a comment
prasadtalasila
left a comment
There was a problem hiding this comment.
@8ohamed please see the comments
| Returns: | ||
| dict: A dictionary containing a timestamp and an 'accel' dictionary with sensor data. | ||
| If more than one sample is collected, the values are averaged. |
Contributor
There was a problem hiding this comment.
Does this mean that the dictionary being returned has only one value per axis irrespective of the number of samples collected?
If so, what happens to the accumulated samples? Are the old samples completely discarded at the end of read() fumction? If not, there is a possibility of growing data points inside Accelerator.
src/cp-sens/methods/pyoma/replace.md
Outdated
Contributor
There was a problem hiding this comment.
There need not be any copying. The idea is to write a thin wrapper around pyoma code to make the needed modifications.
Please identify the changes and write wrappers onlu for them.
|
|
||
| # Simulate message arrival after a short delay. | ||
| def simulate_message(): | ||
| time.sleep(0.1) |
Contributor
There was a problem hiding this comment.
Why is there a 0.1 sec delay?
Contributor
|
@8ohamed please let me know when you are done. Thanks. |
Contributor
prasadtalasila
left a comment
prasadtalasila
left a comment
There was a problem hiding this comment.
@8ohamed Thanks for the updates. Please see the suggestions.
src/cp-sens/data/accel/constants.py
Outdated
Contributor
There was a problem hiding this comment.
please leave a short comment / doc strings for each of these constants.
|
|
||
|
|
||
| class Accelerometer(IAccelerometer): | ||
| def __init__(self, mqtt_client, topic: str = "accelerometer/data", fifo_size: int = 10000, axis: list = ['x', 'y', 'z']): |
Contributor
There was a problem hiding this comment.
Please add MAX_FIFO_SIZE as a constant in constants.py
Contributor
prasadtalasila
left a comment
prasadtalasila
left a comment
There was a problem hiding this comment.
@8ohamed thanks for the updates. Please check the comments.
| """Handles incoming MQTT messages.""" | ||
| future = threading.Thread(target=self._process_message, args=(msg,), daemon=True) | ||
| future.start() | ||
| future.join() # Ensures proper handling of message processing |
Contributor
There was a problem hiding this comment.
Error handling would be good. However, this statement would be blocking the main thread which might not be desirable. The lock forces safe access to the deque anyway.
Collaborator
Author
|
@prasadtalasila I have updated the test so they use real MQTT client. And I updated some parts of the code. |
Contributor
|
Updates to pyproject.toml |
Contributor
|
@8ohamed please see the pr-7-suggestions branch. It fixes the path imports issue. |
Contributor
|
@8ohamed please use the following {
"MQTT": {
"host": "test.mosquitto.org",
"port": 1883,
"userId": "",
"password": "",
"ClientID": "test_client_id",
"QoS": 1,
"TopicsToSubscribe": ["topic"]
}
} |
Collaborator
Author
|
@prasadtalasila I have restructured the project, where I have removed the cp-sens file. Now, the absolute imports work for the tests and the experiment file. I will send the sysid output to Swarup and let you know when he approves it. |
Contributor
|
@8ohamed, please add the following text at the end of ## MQTT Configuration
The acceleration measurements are streamed via MQTT broker. The following
configuration needs to be placed in `config/mqtt.json` and
credentials modified.
```json
{
"MQTT": {
"host": "test.mosquitto.org",
"port": 1883,
"userId": "",
"password": "",
"ClientID": "test_client_id",
"QoS": 1,
"TopicsToSubscribe": [
"cpsens/d8-3a-dd-f5-92-48/cpsns_Simulator/+/acc/raw/data",
"cpsens/d8-3a-dd-f5-92-48/cpsns_Simulator/+/acc/raw/metadata",
"cpsens/d8-3a-dd-37-d3-08/3050-A-060_sn_106209/+/acc/raw/data",
"cpsens/d8-3a-dd-37-d3-08/3050-A-060_sn_106209/+/acc/raw/metadata",
"cpsens/2c-cf-67-25-da-db/mcc172_21C2CCC/+/acc/raw/data",
"cpsens/2c-cf-67-25-da-db/mcc172_21C2CCC/+/acc/raw/metadata",
"cpsens/d8-3a-dd-37-d2-7e/3160-A-042_sn_999998/+/acc/raw/data",
"cpsens/d8-3a-dd-37-d2-7e/3160-A-042_sn_999998/+/acc/raw/metadata"
]
}
} |
Contributor
|
@8ohamed the |
Contributor
|
In addition, the tests fail as well. |
Contributor
|
@8ohamed, please fix the code quality issues. |
Contributor
prasadtalasila
left a comment
prasadtalasila
left a comment
There was a problem hiding this comment.
@8ohamed , thanks for the updates. Please see the comments.
Contributor
There was a problem hiding this comment.
please move this file to test/input_files and use it in a test. Please put a small test/input_files/README.md mentioning the purpose of this directory.
Contributor
There was a problem hiding this comment.
please change the project name to example-shm
Contributor
There was a problem hiding this comment.
please use the mqtt.json file from PR-7-suggestions branch and update the code accordingly.
Collaborator
Author
|
@prasadtalasila For the mqtt.json in PR-7-suggestions branch it is the same one. |
Contributor
|
@8ohamed I pushed some changes directly to this PR branch. The changes are for ignoring pyoma files for linting purposes and minor updates to poetry setup. Please go ahead anf make changes in the |
Collaborator
Author
|
Hi @prasadtalasilaI have updated the process message logic. Now, we append the samples collected from each message to a map, where the key is samples_from_daq_start. It is not being hashed. Now I am working on a new branch where I am writing alligner code, where we can use it for multiple channels/topics for the requested amount. |
Contributor
|
@8ohamed please create new PR with just the code for the Accelerometer without pyoma. Do keep the experiment_1.py to show the results. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.