Added support for PiEEG #723

nickgamb · 2024-05-30T20:46:09Z

Added initial support for PiEEG board.
Updated supported board docs.
Added pieeg_emulator.py.
Added pieeg_server.py and pieeg_client.py examples.

Fixed multiple bugs.

Updated pieeg_board to handle websocket

Updated docs and emulator for pieeg support. Included pieeg_server.py and pieeg_client.py examples in the python_package\examples folder.

nickgamb · 2024-05-30T20:49:16Z

Hi @Andrey1994! This is a draft PR for PiEEG board support. I am new to BrainFlow and it would help me a lot if you could take a peek at this draft when you have a moment. No rush. I want to iterate on this until you feel it is a solid PR. Please let me know what you think!

Andrey1994

hi, good overall, I did initial check. First of all, there are some changes not related to pieeg, they should not be a part of this PR. Also, it should not send any data from brainflow direcrtly using custom implementations, there are methods like add_streamer and streaming_board which do it in a uniform manner. I also dont understand how emulator can work

Andrey1994 · 2024-05-30T22:15:59Z

CMakeLists.txt

@@ -1,70 +1,96 @@
-cmake_minimum_required (VERSION 3.13)
-project (brainflow)


these code style changes are not related to pieeg, and should be reverted

Reverted the code style changes not related to PIEEG as requested.

Andrey1994 · 2024-05-30T22:25:09Z

emulator/brainflow_emulator/pieeg_emulator.py

+
+if __name__ == "__main__":
+    logging.basicConfig(level=logging.INFO)
+    start_emulator("/dev/ttyUSB0")  # Adjust the port as necessary


I dont understand how it can work, since pieeg also uses /dev/gpiochip0 which is not emulated here afaict. Emulator is not a strict requirement, so maybe it can be skipped(remove it if doesnt work, I have no idea how to emulate gpio)

Removed the emulator code as it does not handle GPIO emulation correctly and is not required for this PR.

Andrey1994 · 2024-05-30T22:26:36Z

python_package/brainflow/__init__.py

+from brainflow.utils import *
+
+__version__ = "0.0.1"


version is set in CI scripts, they parse setup.py and override its content. For consistency I would recommend to remove it from here or parse it also in CI and override for publishing

Removed the version setting from init.py to maintain consistency with CI scripts.

Andrey1994 · 2024-05-30T22:29:31Z

python_package/examples/tests/pieeg_client.py

@@ -0,0 +1,83 @@
+import logging
+import socket


we maintain only minimalistic examples, its ok to create smth like brainflow_get_data.py for pieeg only, but stuff like sending data, etc is outside from the scope of these examples, so please remove

Removed pieeg_client.py as it is outside the scope of minimalistic examples maintained by the project.

Andrey1994 · 2024-05-30T22:30:05Z

python_package/examples/tests/pieeg_sever.py

+import threading
+import time
+import logging
+from brainflow.board_shim import BoardShim, BrainFlowInputParams, BoardIds, BrainFlowError


remove it also, and for streaming data between nodes and processes, we have streaming board

Removed pieeg_server.py as data streaming should be handled by high-level APIs like streaming boards or user implementations.

Andrey1994 · 2024-05-30T22:34:57Z

src/board_controller/emotibit/emotibit.cpp

 #include "timestamp.h"

+#ifdef __ANDROID__


not related to pieeg, should be removed

Reverted changes to emotibit.cpp - not related to pieeg.

Andrey1994 · 2024-05-30T22:35:38Z

src/board_controller/pieeg/inc/periphery/CHANGELOG.md

@@ -0,0 +1,184 @@
+* v2.4.2 - 07/05/2023


c_periphery should be in third_party folder

c_periphery moved to third_party folder and build.cmake updated.

Andrey1994 · 2024-05-30T22:37:19Z

src/board_controller/pieeg/inc/pieeg_board.h

+#include "board.h"
+#include "board_controller.h"
+#include "math.h"
+#include "socket_server_tcp.h"


why does it need socket?

Removed the unnecessary socket_server_tcp.h inclusion.

Andrey1994 · 2024-05-30T22:38:02Z

src/board_controller/pieeg/pieeg_board.cpp

+        return (int)BrainFlowExitCodes::UNABLE_TO_OPEN_PORT_ERROR;
+    }
+
+    server_socket = new SocketServerTCP("0.0.0.0", params.ip_port, true);


what is it?

Removed the socket server initialization as data streaming should not be handled here.

Andrey1994 · 2024-05-30T22:40:55Z

src/board_controller/pieeg/pieeg_board.cpp

+            // Send data over TCP
+            if (server_socket && server_socket->client_connected)
+            {
+                server_socket->send(package, num_rows * sizeof(double));


it should not send anything from here, idea of this code is to read data straight on raspberry pi and if its needed to be sent to PC or somewhere use high level API like streaming boards, or user's own implementations

Removed the custom data sending code as streaming should be done via high-level APIs.

- Removed socket-related code from PIEEGBoard to adhere to high-level API usage for data streaming. - Moved the c_periphery library to the third_party folder. - Updated CMakeLists and build.cmake to conditionally include and link periphery only when needed. - Removed unnecessary 'c' and 'm' libraries from the target_link_libraries. - Removed the emulator script and unnecessary examples from the PR.

Reverting emotibit.cpp changes.

nickgamb · 2024-05-31T17:57:52Z

hi, good overall, I did initial check. First of all, there are some changes not related to pieeg, they should not be a part of this PR. Also, it should not send any data from brainflow direcrtly using custom implementations, there are methods like add_streamer and streaming_board which do it in a uniform manner. I also dont understand how emulator can work

Hi @Andrey1994,

Thank you for the review! I have think I have addressed all of the feedback and pushed the necessary changes. The following updates have been made:

Reverted unrelated code style changes in CMakeLists.txt.
Removed emulator code that does not handle GPIO correctly.
Removed version setting from init.py for consistency with CI scripts.
Removed pieeg_client.py and pieeg_server.py as they are outside the scope of minimalistic examples.
Moved c_periphery to third_party folder.
Updated build.cmake to only include necessary libraries and restore proper Android linking.
Removed unnecessary socket includes and custom data sending in pieeg_board.h and pieeg_board.cpp.

Let me know what you think!

Andrey1994

what is the difference between this and implementation which we had? I remember there were issues, did you fix them here or was it in fw/hw? Can you share brainflow_get_data.py data dumped to a file?(use add_streamer("file://data.csv:w") right after prepare_session to store data)

nickgamb · 2024-06-01T03:00:17Z

what is the difference between this and implementation which we had? I remember there were issues, did you fix them here or was it in fw/hw? Can you share brainflow_get_data.py data dumped to a file?(use add_streamer("file://data.csv:w") right after prepare_session to store data)

After reviewing the implementation that you had previously I found that the approach is very similar.

The new version has improved class naming and consistency in logging and error handling.
Both versions handle GPIO and SPI initialization similarly, but the new version ensures better consistency.
The core data reading and streaming logic remain the same, with similar approaches to handling SPI transfers and GPIO events.

I am not able to be at my desk this weekend but I will upload a dump as soon as possible next week. I stream the data to my Android app over a socket so I can confirm that the board is collecting the data and it appears to be functioning as expected. I have more testing to do for sure. If you can tell me more about the issue you had in the past I can attempt to replicate it?

Andrey1994 · 2024-06-01T07:55:22Z

@Ildaron do you remember what was the problem?
I think it was smth about lower sampling rate and lost packages

Ildaron · 2024-06-01T23:45:09Z

Hey @Andrey1994 @nickgamb yes there was definitely a problem with packet loss, I also changed the register values in the latest versions, and now works well , and then I tested Pi3
Now Pi4 and Pi5)

Andrey1994 · 2024-06-02T16:49:30Z

Which registers are changed? Can you check the code here in terms of register writes and suggest changes?

Ildaron · 2024-06-03T15:18:55Z

Hi @Andrey1994 yes of course,
only I didn't find here script with the settings for registers, Hi @nickgamb can you clarify where I need check these parts

write_byte (config1, 0x96)
write_byte (config2, 0xD4)
write_byte (config3, 0xFF)

nickgamb · 2024-06-04T03:25:14Z

Hi @Andrey1994 yes of course, only I didn't find here script with the settings for registers, Hi @nickgamb can you clarify where I need check these parts

write_byte (config1, 0x96) write_byte (config2, 0xD4) write_byte (config3, 0xFF)

I think you are looking for this code @Ildaron ?

brainflow/src/board_controller/pieeg/pieeg_board.cpp

Line 71 in 587c0e5

int PIEEGBoard::start_stream(int buffer_size, const char *streamer_params)

Ildaron · 2024-06-04T09:46:34Z

Hi @Andrey1994 yes of course, only I didn't find here script with the settings for registers, Hi @nickgamb can you clarify where I need check these parts
write_byte (config1, 0x96) write_byte (config2, 0xD4) write_byte (config3, 0xFF)

I think you are looking for this code @Ildaron ?

brainflow/src/board_controller/pieeg/pieeg_board.cpp

Line 71 in 587c0e5

int PIEEGBoard::start_stream(int buffer_size, const char *streamer_params)

Hi @nickgamb need replace only this one
from
spi_res = write_reg(0x03, 0xE0);
to
spi_res = write_reg(0x03, 0xFF); // reg3

Adjusted reg3 from spi_res = write_reg(0x03, 0xE0); to spi_res = write_reg(0x03, 0xFF);

nickgamb · 2024-06-04T20:14:22Z

Hi @Andrey1994 yes of course, only I didn't find here script with the settings for registers, Hi @nickgamb can you clarify where I need check these parts
write_byte (config1, 0x96) write_byte (config2, 0xD4) write_byte (config3, 0xFF)

I think you are looking for this code @Ildaron ?

brainflow/src/board_controller/pieeg/pieeg_board.cpp

Line 71 in 587c0e5

int PIEEGBoard::start_stream(int buffer_size, const char *streamer_params)

Hi @nickgamb need replace only this one from spi_res = write_reg(0x03, 0xE0); to spi_res = write_reg(0x03, 0xFF); // reg3

Fantastic! Thank you so much @Ildaron. I have made update.

Andrey1994 · 2024-06-04T20:56:46Z

Good, could you share 10 seconds of recordings?

Andrey1994 · 2024-06-09T21:18:27Z

any updates? can any of you run the basic brainflow get data script and store data in a file?

nickgamb · 2024-06-10T15:06:16Z

any updates? can any of you run the basic brainflow get data script and store data in a file?

Hey @Andrey1994 . I have not forgotten about this and I appreciate your patience. I took a tech break for the weekend to spend time with my family. I also am waiting for a new set of electrodes from OpenBCI. I was using some equipment on loan to get started and needed to buy my own. They shipped last week but tracking is showing delivery to me on Wednesday. If you can hold for a little longer I will plug the new electrodes in as soon as I open the package and run the test.

Adjusted CMakeLists.txt and board__controller build.cmake to properly build periphery and link when defined.

nickgamb · 2024-06-13T19:55:02Z

any updates? can any of you run the basic brainflow get data script and store data in a file?

Hey @Andrey1994 . I have not forgotten about this and I appreciate your patience. I took a tech break for the weekend to spend time with my family. I also am waiting for a new set of electrodes from OpenBCI. I was using some equipment on loan to get started and needed to buy my own. They shipped last week but tracking is showing delivery to me on Wednesday. If you can hold for a little longer I will plug the new electrodes in as soon as I open the package and run the test.

Hi @Andrey1994. Thank you for your patience. I seem to have not received ear clips so I am stuck using 2 of my signal electrodes for ref and biasout at the moment. That means I only have 6 channels for signal. I am buying what I need but was able to pull this data for you to show the progress.
eeg_data.zip

Again this is only 6 channels with 2 flat ThinkPulse electrodes operating as temp ref and biasout. Please let me know if you are seeing anything. This was not taken in a great environment so there is noise but I think it should be functioning as expected considering.

Andrey1994 · 2024-06-13T20:10:13Z

Thanks, I will review everything tomorrow!

Andrey1994 · 2024-06-30T00:49:50Z

Moved that to #729

nickgamb added 4 commits May 29, 2024 22:24

Updates for PiEEG Board

78d466e

Fixed multiple bugs.

including c-periphery

d71c0aa

Updated pieeg_board to handle websocket

6c4b400

Updated pieeg_board to handle websocket

Updated docs and emulator for pieeg support

29e575b

Updated docs and emulator for pieeg support. Included pieeg_server.py and pieeg_client.py examples in the python_package\examples folder.

Andrey1994 requested changes May 30, 2024

View reviewed changes

nickgamb added 2 commits May 31, 2024 10:42

Update emotibit.cpp

2959c4d

Reverting emotibit.cpp changes.

nickgamb marked this pull request as ready for review May 31, 2024 17:58

nickgamb requested a review from Andrey1994 May 31, 2024 23:31

Andrey1994 reviewed May 31, 2024

View reviewed changes

Updates to pieeg_board.cpp

acdda10

Adjusted reg3 from spi_res = write_reg(0x03, 0xE0); to spi_res = write_reg(0x03, 0xFF);

Adjustments to cmake

9a49ca9

Adjusted CMakeLists.txt and board__controller build.cmake to properly build periphery and link when defined.

Andrey1994 mentioned this pull request Jun 30, 2024

restore support for pieeg #729

Merged

Andrey1994 closed this Jun 30, 2024

		@@ -1,70 +1,96 @@
		cmake_minimum_required (VERSION 3.13)
		project (brainflow)

Added support for PiEEG #723

Added support for PiEEG #723

Conversation

nickgamb commented May 30, 2024

nickgamb commented May 30, 2024

Andrey1994 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nickgamb commented May 31, 2024

Andrey1994 left a comment

Choose a reason for hiding this comment

nickgamb commented Jun 1, 2024

Andrey1994 commented Jun 1, 2024

Ildaron commented Jun 1, 2024 • edited Loading

Andrey1994 commented Jun 2, 2024

Ildaron commented Jun 3, 2024

nickgamb commented Jun 4, 2024

Ildaron commented Jun 4, 2024 • edited Loading

nickgamb commented Jun 4, 2024

Andrey1994 commented Jun 4, 2024

Andrey1994 commented Jun 9, 2024

nickgamb commented Jun 10, 2024

nickgamb commented Jun 13, 2024

Andrey1994 commented Jun 13, 2024

Andrey1994 commented Jun 30, 2024

Ildaron commented Jun 1, 2024 •

edited

Loading

Ildaron commented Jun 4, 2024 •

edited

Loading