hasslesstech/IoT-Systems
1
0
Fork 0
Code Issues Pull Requests Actions Activity

FileReader #5

Merged
bacant150 merged 15 commits from lab1_huranets into lab1 2026-02-26 10:04:10 +02:00
Conversation 34 Commits 15 Files Changed 3 +10 -21
1 changed files with 10 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 3e0b4762ef - Show all commits

31
agent/src/file_datasource.py
View File

@@ -1,5 +1,3 @@
from __future__ import annotations
import csv import csv
hasslesstech commented 2026-02-24 19:37:02 +02:00 (Migrated from github.com)
Review
Copy Link

Naming convention is misleading, suggests parser specifically takes 16-bit integers. This parser should read any integer value and should just be int(row[0]), not a separate function call

Several identical issues occur in the next few lines

Naming convention is misleading, suggests parser specifically takes 16-bit integers. This parser should read any integer value and should just be `int(row[0])`, not a separate function call Several identical issues occur in the next few lines
hasslesstech commented 2026-02-24 19:37:45 +02:00 (Migrated from github.com)
Review
Copy Link

Unused variables, seem to be leftovers from previously removed INT16 boundary checking

Unused variables, seem to be leftovers from previously removed INT16 boundary checking
bacant150 commented 2026-02-24 20:01:55 +02:00 (Migrated from github.com)
Review
Copy Link

removed unnecessary variables

removed unnecessary variables
hasslesstech commented 2026-02-24 20:04:13 +02:00 (Migrated from github.com)
Review
Copy Link

Method is not used and does not provide additional functionality compared to default int() function, so should be removed

Method is not used and does not provide additional functionality compared to default `int()` function, so should be removed
bacant150 commented 2026-02-24 20:23:35 +02:00 (Migrated from github.com)
Review
Copy Link

Fixed: removed int16-specific naming/helpers and now parse accelerometer values directly using int(row[i]) without any bit-width assumptions. Pushed the update to the PR.

Fixed: removed int16-specific naming/helpers and now parse accelerometer values directly using int(row[i]) without any bit-width assumptions. Pushed the update to the PR.
bacant150 commented 2026-02-24 20:24:06 +02:00 (Migrated from github.com)
Review
Copy Link

Removed the unused _parse_int helper (it didn’t add any functionality over built-in int()). Pushed the update to the PR.

Removed the unused _parse_int helper (it didn’t add any functionality over built-in int()). Pushed the update to the PR.
VladiusVostokus commented 2026-02-25 11:17:41 +02:00 (Migrated from github.com)
Review
Copy Link

And after chage of rewinders, you can remove _acc_buf to hold first row

And after chage of rewinders, you can remove _acc_buf to hold first row
VladiusVostokus commented 2026-02-25 11:24:43 +02:00 (Migrated from github.com)
Review
Copy Link

What does this means?
if assuming this is a header with indexies of domain objects maybe make sense.

What does this means? if assuming this is a header with indexies of domain objects maybe make sense.
bacant150 commented 2026-02-25 12:14:42 +02:00 (Migrated from github.com)
Review
Copy Link

Removed unnecessary buffers like _acc_buf and _gps_buf.

Removed unnecessary buffers like _acc_buf and _gps_buf.
bacant150 commented 2026-02-25 12:53:57 +02:00 (Migrated from github.com)
Review
Copy Link

I initially tried removing this part, but the testing failed without it

I initially tried removing this part, but the testing failed without it
import time import time
from datetime import datetime from datetime import datetime
@@ -29,9 +27,6 @@ class FileDatasource:
self._acc_buf: Optional[List[str]] = None self._acc_buf: Optional[List[str]] = None
self._gps_buf: Optional[List[str]] = None self._gps_buf: Optional[List[str]] = None
self._acc_has_header: Optional[bool] = None
self._gps_has_header: Optional[bool] = None
def startReading(self, *args, **kwargs): def startReading(self, *args, **kwargs):
"""Must be called before read()""" """Must be called before read()"""
if self._started: if self._started:
@@ -80,16 +75,17 @@ class FileDatasource:
self._acc_f = open(self.accelerometer_filename, "r", newline="", encoding="utf-8") self._acc_f = open(self.accelerometer_filename, "r", newline="", encoding="utf-8")
VladiusVostokus commented 2026-02-25 11:20:05 +02:00 (Migrated from github.com)
Review
Copy Link

You just can not return this variable from _detect_header_and_buffer() if it is not used

You just can not return this variable from _detect_header_and_buffer() if it is not used
bacant150 commented 2026-02-25 12:36:28 +02:00 (Migrated from github.com)
Review
Copy Link

removed unnecessary variables from _detect_header_and_buffer() and fixed the _rewind_acc() and _rewind_gps() methods

removed unnecessary variables from _detect_header_and_buffer() and fixed the _rewind_acc() and _rewind_gps() methods
self._gps_f = open(self.gps_filename, "r", newline="", encoding="utf-8") self._gps_f = open(self.gps_filename, "r", newline="", encoding="utf-8")
self._acc_reader = csv.reader(self._acc_f) self._acc_reader = csv.reader(self._acc_f, skipinitialspace=True)
VladiusVostokus commented 2026-02-25 16:05:28 +02:00 (Migrated from github.com)
Review
Copy Link

if you open file, your read pointer is already 0, why to set it to 0 again?

if you open file, your read pointer is already 0, why to set it to 0 again?
bacant150 commented 2026-02-25 21:09:54 +02:00 (Migrated from github.com)
Review
Copy Link

I removed the unnecessary rewind from _open_files().

I removed the unnecessary rewind from _open_files().
self._gps_reader = csv.reader(self._gps_f) self._gps_reader = csv.reader(self._gps_f, skipinitialspace=True)
self._acc_buf = None self._acc_buf = None
self._gps_buf = None self._gps_buf = None
self._acc_has_header, self._acc_buf = self._detect_header_and_buffer( # detect header / buffer first data row (we only need the buffered row)
_, self._acc_buf = self._detect_header_and_buffer(
self._acc_reader, expected_cols=3, header_tokens=("x", "y", "z") self._acc_reader, expected_cols=3, header_tokens=("x", "y", "z")
) )
self._gps_has_header, self._gps_buf = self._detect_header_and_buffer( _, self._gps_buf = self._detect_header_and_buffer(
self._gps_reader, expected_cols=2, header_tokens=("longitude", "latitude") self._gps_reader, expected_cols=2, header_tokens=("longitude", "latitude")
) )
@@ -107,15 +103,13 @@ class FileDatasource:
self._gps_reader = None self._gps_reader = None
self._acc_buf = None self._acc_buf = None
VladiusVostokus commented 2026-02-25 16:06:38 +02:00 (Migrated from github.com)
Review
Copy Link

Why leave _ here if it's not used?

Why leave _ here if it's not used?
bacant150 commented 2026-02-25 21:13:00 +02:00 (Migrated from github.com)
Review
Copy Link

Good catch. The _detect_header_and_buffer helper wasn’t used and didn’t modify class state, so I removed it.

Good catch. The _detect_header_and_buffer helper wasn’t used and didn’t modify class state, so I removed it.
self._gps_buf = None self._gps_buf = None
self._acc_has_header = None
self._gps_has_header = None
def _rewind_acc(self) -> None: def _rewind_acc(self) -> None:
if self._acc_f is None: if self._acc_f is None:
raise RuntimeError("Accelerometer file is not open.") raise RuntimeError("Accelerometer file is not open.")
self._acc_f.seek(0) self._acc_f.seek(0)
self._acc_reader = csv.reader(self._acc_f) self._acc_reader = csv.reader(self._acc_f, skipinitialspace=True)
VladiusVostokus commented 2026-02-25 11:14:51 +02:00 (Migrated from github.com)
Review
Copy Link

you can just skip first row after seek(0) knowing that this is a header, not actual value,
so you can write like
self._acc_f.seek(0)
self._acc_reader = csv.reader(self._acc_f)
next(self._acc_reader)
row = next(self.acc_reader)

you can just skip first row after seek(0) knowing that this is a header, not actual value, so you can write like self._acc_f.seek(0) self._acc_reader = csv.reader(self._acc_f) next(self._acc_reader) row = next(self.acc_reader)
VladiusVostokus commented 2026-02-25 11:15:57 +02:00 (Migrated from github.com)
Review
Copy Link

And do the same for other rewinders

And do the same for other rewinders
bacant150 commented 2026-02-25 12:02:34 +02:00 (Migrated from github.com)
Review
Copy Link

Refactoring file rewind logic to skip header line after seek(0) and for other rewinders

Refactoring file rewind logic to skip header line after seek(0) and for other rewinders
self._acc_has_header, self._acc_buf = self._detect_header_and_buffer( _, self._acc_buf = self._detect_header_and_buffer(
self._acc_reader, expected_cols=3, header_tokens=("x", "y", "z") self._acc_reader, expected_cols=3, header_tokens=("x", "y", "z")
) )
VladiusVostokus commented 2026-02-25 15:52:46 +02:00 (Migrated from github.com)
Review
Copy Link

Why leave _ here if it's not used?
Maybe dont return anything from this method?

Why leave _ here if it's not used? Maybe dont return anything from this method?
VladiusVostokus commented 2026-02-25 16:00:51 +02:00 (Migrated from github.com)
Review
Copy Link

As I see this method doesn't modify any class fiels, so maybe just remove this method?

As I see this method doesn't modify any class fiels, so maybe just remove this method?
bacant150 commented 2026-02-25 20:57:57 +02:00 (Migrated from github.com)
Review
Copy Link

The _detect_header_and_buffer method was not modifying any class state and its return value was unused.
I removed the method and simplified the logic.

The _detect_header_and_buffer method was not modifying any class state and its return value was unused. I removed the method and simplified the logic.
@@ -123,8 +117,8 @@ class FileDatasource:
if self._gps_f is None: if self._gps_f is None:
raise RuntimeError("GPS file is not open.") raise RuntimeError("GPS file is not open.")
self._gps_f.seek(0) self._gps_f.seek(0)
self._gps_reader = csv.reader(self._gps_f) self._gps_reader = csv.reader(self._gps_f, skipinitialspace=True)
self._gps_has_header, self._gps_buf = self._detect_header_and_buffer( _, self._gps_buf = self._detect_header_and_buffer(
self._gps_reader, expected_cols=2, header_tokens=("longitude", "latitude") self._gps_reader, expected_cols=2, header_tokens=("longitude", "latitude")
) )
@@ -144,7 +138,6 @@ class FileDatasource:
self._rewind_acc() self._rewind_acc()
continue continue
row = [c.strip() for c in row]
if not row or not any(row): if not row or not any(row):
continue continue
@@ -166,7 +159,6 @@ class FileDatasource:
self._rewind_gps() self._rewind_gps()
continue continue
row = [c.strip() for c in row]
if not row or not any(row): if not row or not any(row):
continue continue
@@ -182,7 +174,6 @@ class FileDatasource:
first = next(rdr, None) first = next(rdr, None)
if first is None: if first is None:
return False, None return False, None
first = [c.strip() for c in first]
if first and any(first): if first and any(first):
break break
@@ -209,9 +200,7 @@ class FileDatasource:
y = int(row[1]) y = int(row[1])
z = int(row[2]) z = int(row[2])
except ValueError as e: except ValueError as e:
hasslesstech commented 2026-02-24 15:25:56 +02:00 (Migrated from github.com)
Review
Copy Link

This value range check might break future integration with higher precision accelerometer data, hence should be removed

This value range check might break future integration with higher precision accelerometer data, hence should be removed
raise ValueError( raise ValueError(f"Invalid accelerometer values (expected integers): {row}") from e
f"Invalid accelerometer values (expected integers): {row}"
) from e
return Accelerometer(x=x, y=y, z=z) return Accelerometer(x=x, y=y, z=z)