Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trik Studio 3D Visualization #1526

Open
wants to merge 32 commits into
base: master
Choose a base branch
from

Conversation

ladaegorova18
Copy link

No description provided.

@@ -222,3 +227,7 @@ SOURCES += \

FORMS += \
$$PWD/src/engine/view/twoDModelWidget.ui \

DISTFILES += \
$$PWD/src/engine/trajectory/frames.proto
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

мусор

@@ -13,6 +13,7 @@
# limitations under the License.

QT += widgets xml svg
QT += network
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Зачем отдельная строка?

{
if (connToVizualizator == nullptr) {
connToVizualizator = new ConnectionToVizualizator();
connToVizualizator->setPort(8080);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Порт должен быть в опциях, хотя бы в Settings.


/// saving it to file
QFile file("trajectory.json");
if( file.open( QIODevice::WriteOnly | QIODevice::Text | QIODevice::Truncate ) ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Нет обработки ошибки, хотя бы QLOG_ERROR () <<


try {
sendTrajectory(data);
} catch (exception e) {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Исключения надо ловить по ссылке. Но это дурной тон ловить std::exception

TrajectorySaver::TrajectorySaver(QObject *parent)
: QObject(parent)
{
connToVizualizator = new ConnectionToVizualizator();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Кто и когда удаляет?

QTcpSocket *socket;
QTimer *keepaliveTimer;

QString Ip {"10.0.5.2"};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Параметры сетевого соединения должны быть в настройках


public:
// ConnectionToVizualizator(QString ip, int port);
ConnectionToVizualizator();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Возможно, конструктору нужен параметр QObject *parent, раз уж это Q_OBJECT

#include <QTimer>

/// Connection to Unity to send frames and run/stop/restart
class alignas(8) ConnectionToVizualizator : public QObject
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А зачем тут alignas(8)? Такой код нельзя без комментария.


private:
QTcpSocket *socket;
QTimer *keepaliveTimer;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В продукте принята схема именования полей с префиксом, то есть mSocket и подобное.

Copy link
Contributor

@IKhonakhbeeva IKhonakhbeeva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RobotModel::deserialize и serialize должны выглядеть как "до изменений" (то есть после моего пулреквеста с форматом сейва). Ни на что в этом пул реквесте это на первый поверхностный вгляд повлиять не должно

После этого билды должны починиться

@@ -28,6 +28,7 @@
#include "twoDModel/engine/twoDModelDisplayWidget.h"

#include "twoDModel/twoDModelDeclSpec.h"
#include "plugins/robots/common/twoDModel/src/engine/trajectory/connectionToVizualizator.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Зачем этот #include?

@@ -108,6 +113,7 @@ class TWO_D_MODEL_EXPORT TwoDModelWidget : public QWidget
QDomDocument generateWorldModelWithBlobsXml() const;
QDomDocument generateWorldModelXml() const;
QDomDocument generateBlobsXml() const;
ConnectionToVizualizator *mConnToVizualizator;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Отсутствует инициализация {}, не указано, кто владеет объектом

@@ -44,6 +45,7 @@ Box2DPhysicsEngine::Box2DPhysicsEngine (const WorldModel &worldModel
, mWorld(new b2World(b2Vec2(0, 0)))
, mPrevPosition(b2Vec2(0, 0))
, mPrevAngle(0)
, mTrajSaver(new TrajectorySaver(nullptr))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Думаю, что тут надо бы не nullptr

@@ -353,7 +365,10 @@ void Box2DPhysicsEngine::nextFrame()
item->setPos(scenePos - item->boundingRect().center());
item->setRotation(angleToScene(mBox2DDynamicItems[item]->getRotation()));
}
auto *abstractItem = dynamic_cast<graphicsUtils::AbstractItem *>(item);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

qobject_cast не подойдёт?

@@ -0,0 +1,166 @@
/* Copyright 2022 Lada Egorova
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

По-моему, тут где-то обычно запятая есть. Но хоть так.

@@ -140,24 +147,50 @@
<string>Network Settings</string>
</property>
<layout class="QGridLayout" name="gridLayout_5">
<item row="0" column="2">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не надо перетаскивать элементы в XML, это лишь увеличивает diff

@ladaegorova18 ladaegorova18 changed the title Trik Studio 3D Visualisation Trik Studio 3D Visualization May 11, 2022
lada and others added 7 commits May 12, 2022 20:37

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@iakov iakov force-pushed the master branch 8 times, most recently from cd7a4ab to ff6360f Compare February 11, 2023 17:45
@iakov iakov force-pushed the master branch 3 times, most recently from dde4319 to 06af258 Compare July 20, 2023 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants