Func did for i2c by Wiktor-Laska · Pull Request #290 · Simba-Avionic/srp

Wiktor-Laska · 2026-01-13T18:38:47Z

funkcje did read, wirte dla zadania #214

Mateusz-Krajewski

Strasznie dużo niespójności, kody błędów są losowe, czasami błędy zwracają brak błędu. Brak zachowanej konwencji plików .h .cpp ? i2c_mw_read ma .h i .cpp a i2c_mw_write ma tylko .h ??? Nie zaimplementowano wszystkich funkcjonalności oraz zepsuto funkcjonalność i2c_service. Dodatkowo jest konflikt repozytorium system_definitions

Mateusz-Krajewski · 2026-01-15T12:04:35Z

+}
+
+} // namespace mw
+} // namespace srp


Suggested change

} // namespace srp

} // namespace srp

Mateusz-Krajewski · 2026-01-15T12:04:55Z

+  ara::core::Result<ara::diag::OperationOutput> Read() noexcept override {
+    return ara::diag::MakeErrorCode(ara::diag::UdsDiagErrc::kRequestOutOfRange);
+  }


Suggested change

ara::core::Result<ara::diag::OperationOutput> Read() noexcept override {

return ara::diag::MakeErrorCode(ara::diag::UdsDiagErrc::kRequestOutOfRange);

}

ara::core::Result<ara::diag::OperationOutput> Read() noexcept override;

Mateusz-Krajewski · 2026-01-15T12:05:27Z

+ara::core::Result<void> I2CMWREAD::Write(const std::vector<uint8_t> &payload) noexcept {
+    if (payload.size() < 3 || !payload.size()%2) {
+        ara::log::LogInfo() << "Wrong payload size: " << payload.size();
+        return ara::diag::MakeErrorCode(ara::diag::UdsDiagErrc::kSubFunctionNotSupported);
+    }
+    if (this->i2c_->Ioctl(payload[0]) != core::ErrorCode::kOk) {
+        ara::log::LogInfo() << "Wrong Input for payload i2c read";
+      return {};
+    }
+
+    ara::log::LogInfo() << "Receive diag write req, calling I2CService::WriteRead";
+    std::vector<uint8_t> vec(payload.begin() + 1, payload.end());
+    auto result = i2c_service_->WriteRead(vec, nullptr);
+
+    if (result == std::nullopt) {
+        ara::log::LogInfo() << "Wrong Input for i2c WriteRead";
+        return ara::diag::MakeErrorCode(ara::diag::UdsDiagErrc::kConditionsNotCorrect);
+    }
+
+    return {};
+}


I jak mam odczytać co zwróciła funkcja? chyba metoda read powinna zwracac wynik ostatniej operacji?

Mateusz-Krajewski · 2026-01-15T12:05:53Z

+ara::core::Result<void> I2CMWREAD::Write(const std::vector<uint8_t> &payload) noexcept {
+    if (payload.size() < 3 || !payload.size()%2) {
+        ara::log::LogInfo() << "Wrong payload size: " << payload.size();
+        return ara::diag::MakeErrorCode(ara::diag::UdsDiagErrc::kSubFunctionNotSupported);


Suggested change

return ara::diag::MakeErrorCode(ara::diag::UdsDiagErrc::kSubFunctionNotSupported);

return ara::diag::MakeErrorCode(ara::diag::UdsDiagErrc::kInvalidMessageLengthFormat);

Mateusz-Krajewski · 2026-01-15T12:06:48Z

+    }
+    if (this->i2c_->Ioctl(payload[0]) != core::ErrorCode::kOk) {
+        ara::log::LogInfo() << "Wrong Input for payload i2c read";
+      return {};


Suggested change

return {};

return ara::diag::MakeErrorCode(ara::diag::UdsDiagErrc::kRequestSequenceError);

ten błąd tyczy się sytułacji ze w zlej kolejnosci przysyłasz dane powinien być bład o długości zapytania jest takie

Mateusz-Krajewski · 2026-01-15T12:09:58Z

+    if (this->i2c_->Write(vec) == core::ErrorCode::kOk) {
+      ara::log::LogInfo() << "Wrong Input for i2c Write";
+      return ara::diag::MakeErrorCode(
+        ara::diag::UdsDiagErrc::kSubFunctionNotSupported);


?? CHATGPT PLS GIVE ME RANDOM ERROR CODE

użyłem błędu z funkcji wyżej "by działało" co podałeś

Mateusz-Krajewski · 2026-01-15T12:10:20Z

 I2CService::I2CService():
-    i2c_logger_(ara::log::LoggingMenager::GetInstance()->CreateLogger("i2c", "", ara::log::LogLevel::kInfo)) {}
-
+    i2c_logger_(ara::log::LoggingMenager::GetInstance()->CreateLogger("i2c", "", ara::log::LogLevel::kInfo)),did_instance("/srp/mw/i2cMWService/i2c_Write_did"), read_instance("/srp/mw/i2cMWService/i2c_Read_did") {}


Suggested change

i2c_logger_(ara::log::LoggingMenager::GetInstance()->CreateLogger("i2c", "", ara::log::LogLevel::kInfo)),did_instance("/srp/mw/i2cMWService/i2c_Write_did"), read_instance("/srp/mw/i2cMWService/i2c_Read_did") {}

i2c_logger_(ara::log::LoggingMenager::GetInstance()->CreateLogger("i2c", "", ara::log::LogLevel::kInfo)), did_instance(I2C_WRITE_DID_INSTANCE ), read_instance(I2C_READ_DID_INSTANCE) {}

Mateusz-Krajewski · 2026-01-15T12:11:27Z

@@ -22,8 +22,7 @@ namespace {
    const constexpr char* I2C_IPC_ADDR = "SRP.I2C";


Suggested change

const constexpr char* I2C_IPC_ADDR = "SRP.I2C";

const constexpr char* I2C_IPC_ADDR = "SRP.I2C";

constexpr auto I2C_WRITE_DID_INSTANCE = "/srp/mw/i2cMWService/i2c_Write_did";

constexpr auto I2C_READ_DID_INSTANCE = "/srp/mw/i2cMWService/i2c_Read_did";

Mateusz-Krajewski · 2026-01-15T12:11:56Z

    return std::nullopt;
  }
-  return i2c_->ReadWrite(payload[0], payload[1]);
+  return i2c_->ReadWrite(payload[0]);


czemu? psuje to chyba wszystkie drivery do i2c.

Suggested change

return i2c_->ReadWrite(payload[0]);

return i2c_->ReadWrite(payload[0], payload[1]);

Mateusz-Krajewski · 2026-01-15T12:15:02Z

      const std::shared_ptr<srp::i2c::Header> headerPtr,
      std::optional<std::vector<uint8_t>> payload);
-
+friend class I2CMWREAD;


Nie prościej było używać i2c_controllera?

możliwe, choć nie wiedziałem że jest taka opcja

Mateusz-Krajewski · 2026-01-15T12:30:03Z

Dodatkowo jak widać już sam cpplint się wywala.
Kusi zacytować klasyka: https://youtu.be/CvarySY_6Ic?si=4huUu9sPOOmwdL6Z

Wiktor-Laska · 2026-01-19T17:45:11Z

Strasznie dużo niespójności, kody błędów są losowe, czasami błędy zwracają brak błędu. Brak zachowanej konwencji plików .h .cpp ? i2c_mw_read ma .h i .cpp a i2c_mw_write ma tylko .h ??? Nie zaimplementowano wszystkich funkcjonalności oraz zepsuto funkcjonalność i2c_service. Dodatkowo jest konflikt repozytorium system_definitions

bartoszsnieg · 2026-01-20T19:54:22Z

+        "//deployment/system_definition/diag/jobs/i2c_service/i2c_write:i2c_Write_job",
+        "//deployment/system_definition/diag/jobs/i2c_service/i2c_read:i2c_Read_job",


NIe żeby coś ale DiD natywnie wspiera write i read

Wiktor-Laska added 4 commits December 15, 2025 21:47

did-Write dla i2c

21cbf39

dop

2f3d297

Dodanie funkcji did i2c_mw_read.cpp i i2c_mw_read.h do serwisu I2C

83d0004

system_definition

289281b

Wiktor-Laska requested a review from a team as a code owner January 13, 2026 18:38

Wiktor-Laska requested a review from bartoszsnieg January 13, 2026 18:38

Mateusz-Krajewski requested changes Jan 15, 2026

View reviewed changes

Mateusz-Krajewski force-pushed the func-did-for-i2c branch 2 times, most recently from d6bbd80 to 289281b Compare January 15, 2026 12:25

.

8bc5562

Mateusz-Krajewski linked an issue Jan 15, 2026 that may be closed by this pull request

Write DID/RID for i2c_mw #214

Open

2 tasks

Mateusz-Krajewski assigned Wiktor-Laska Jan 15, 2026

Wiktor-Laska closed this Jan 19, 2026

Wiktor-Laska reopened this Jan 19, 2026

bartoszsnieg reviewed Jan 20, 2026

View reviewed changes

	return ara::diag::MakeErrorCode(ara::diag::UdsDiagErrc::kSubFunctionNotSupported);
	return ara::diag::MakeErrorCode(ara::diag::UdsDiagErrc::kInvalidMessageLengthFormat);

	return {};
	return ara::diag::MakeErrorCode(ara::diag::UdsDiagErrc::kRequestSequenceError);

	i2c_logger_(ara::log::LoggingMenager::GetInstance()->CreateLogger("i2c", "", ara::log::LogLevel::kInfo)),did_instance("/srp/mw/i2cMWService/i2c_Write_did"), read_instance("/srp/mw/i2cMWService/i2c_Read_did") {}
	i2c_logger_(ara::log::LoggingMenager::GetInstance()->CreateLogger("i2c", "", ara::log::LogLevel::kInfo)), did_instance(I2C_WRITE_DID_INSTANCE ), read_instance(I2C_READ_DID_INSTANCE) {}

		@@ -22,8 +22,7 @@ namespace {
		const constexpr char* I2C_IPC_ADDR = "SRP.I2C";

	return i2c_->ReadWrite(payload[0]);
	return i2c_->ReadWrite(payload[0], payload[1]);

		"//deployment/system_definition/diag/jobs/i2c_service/i2c_write:i2c_Write_job",
		"//deployment/system_definition/diag/jobs/i2c_service/i2c_read:i2c_Read_job",

Conversation

Wiktor-Laska commented Jan 13, 2026

Uh oh!

Mateusz-Krajewski left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mateusz-Krajewski commented Jan 15, 2026

Uh oh!

Wiktor-Laska commented Jan 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Mateusz-Krajewski left a comment •

edited

Loading