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

doc: update document especially available_ecu_ids #248

Merged
merged 3 commits into from
Aug 1, 2023
Merged

Conversation

obi-t4
Copy link
Contributor

@obi-t4 obi-t4 commented Jul 31, 2023

Description

Update document especially availale_ecu_ids.
Also docs/SERVICES.md has been updated according to DEVELOPMENT.md

Check list

N/A (no test are needed)

  • test file that covers the bug case(s) is implemented.
  • local test is passed.

Bug fix

Current behavior

Current specification for available_ecu_ids is unclear.

Behaivor after fix

This PR adds more descriptions.
And adds the description who need to filtering out the update reqest.

Related links & ticket

@obi-t4 obi-t4 requested a review from Bodong-Yang July 31, 2023 07:02
@github-actions
Copy link
Contributor

github-actions bot commented Jul 31, 2023

Coverage

Coverage Report
FileStmtsMissCoverMissing
otaclient/app
   __init__.py00100% 
   __main__.py330%16–19
   common.py2681594%55, 103, 185, 200, 275–277, 287, 296–298, 404, 416, 535–539
   configs.py710100% 
   copy_tree.py81396%45, 96, 125
   downloader.py2704484%69, 82–83, 300, 305, 309, 327–328, 378–382, 401–411, 436–454, 463, 538–540, 556, 576–594
   ecu_info.py59788%78–79, 92, 97, 120–121, 130
   errors.py179597%55, 58, 116, 119, 139
   interface.py17476%32, 43, 47, 51
   log_setting.py26773%27–28, 55–65
   main.py31294%43–44
   ota_client.py3188773%69–70, 92, 198–222, 259–261, 271–276, 286–288, 331–332, 337–344, 357–371, 377–380, 405–429, 443–448, 457, 460–468, 547, 552–570
   ota_client_call.py38684%42–44, 80–82
   ota_client_service.py29390%58–60
   ota_client_stub.py3437578%81–90, 99–101, 106, 138, 165, 168, 250, 270, 272, 296, 341, 402, 470–471, 498, 505, 525–536, 540–554, 558–561, 613, 700–709, 741–791, 794
   ota_metadata.py3143190%145, 150, 186–187, 197–201, 213, 271, 304–306, 323–326, 406, 409, 417–419, 432, 441–446, 719–730
   ota_status.py15287%36, 44
   proxy_info.py51786%82–87, 122–129, 137–138
   update_stats.py105298%162, 172
otaclient/app/boot_control
   __init__.py40100% 
   _cboot.py28613951%91–99, 108–121, 127–130, 134, 141, 145, 149, 153–157, 161–165, 170–189, 192–235, 241, 244, 247, 250, 253, 256, 259, 262–263, 266–267, 270–274, 277–278, 293, 296, 329, 356–359, 369–372, 380, 419, 436–439, 448–451, 456–458, 467–473, 476, 483, 513–515, 552–554, 557–567, 570–575
   _common.py36315557%65–74, 83–84, 88–89, 97–98, 109–110, 115–120, 125–130, 139, 148, 152, 156, 160, 174–177, 187–191, 196, 200, 210–214, 222–234, 245–249, 253–257, 269–271, 283–285, 296–310, 320–341, 360–376, 393–404, 412–418, 467, 489–496, 518–539, 577–578, 632, 636–637, 640–648, 652–655, 705–706, 709, 723–731, 746–747, 816–819, 840–843, 856–857, 866
   _errors.py471960%43–69, 89–90
   _grub.py4029078%202, 224, 241–244, 250–254, 280–281, 288–308, 318–320, 328–330, 334, 337, 340, 343, 365–366, 384–385, 433, 468, 488–493, 506, 533–542, 679–681, 708–711, 720, 759–763, 807, 814, 833–836, 843–846, 849, 856, 883–885, 906–908, 911–916, 919–924
   _rpi_boot.py26012153%89–91, 97–152, 176–178, 184–186, 199–201, 207–209, 222–243, 248, 252, 256, 260, 294, 324, 327–329, 337, 346–348, 358–361, 365–372, 451–455, 474–477, 482, 485, 506–511, 514–522, 525–533, 545–548, 552–554, 557
   configs.py57296%46–47
   firmware.py32584%63, 65, 76–78
   protocol.py28582%40, 44, 48, 52, 60
   selecter.py392731%45–69, 77–94
otaclient/app/create_standby
   __init__.py12558%30–36
   common.py2164579%74, 77–78, 82–95, 141, 189–197, 200–203, 207, 218, 292–300, 312, 356–361, 377–378, 392–396, 421–422
   interface.py19384%55, 59, 63
   rebuild_mode.py99991%83–101, 128
otaclient/app/proto
   __init__.py31390%37, 44–45
   _common.py4194988%85, 163, 170, 182–184, 203, 208, 219, 256, 262, 267, 298, 302, 306, 363, 380, 403, 464, 471, 474, 494, 501, 503, 529, 535, 538, 540, 565, 571, 574, 576, 605, 609, 611, 625, 642, 670, 673, 677, 680, 708, 714, 761–766, 797
   _ota_metafiles_wrapper.py841385%37, 40–42, 112–116, 122–125
   _otaclient_v2_pb2_wrapper.py2763687%87, 90–93, 175, 183, 197, 207, 210–213, 258, 261, 264–265, 285, 305, 385, 452, 505, 513–515, 519–522, 525–530, 551, 559, 573, 581, 595
   streamer.py43881%32, 47, 65–66, 71, 80–81, 99
   wrapper.py40100% 
otaclient/ota_proxy
   __init__.py290100% 
   __main__.py21210%16–71
   cache_control.py84989%50, 75, 79, 95, 117, 125, 141, 147–148
   config.py160100% 
   db.py1361291%66, 76–79, 108–110, 129, 162, 171–172, 187, 214
   errors.py100100% 
   orm.py1151091%37, 91, 96, 101, 107, 113, 140–141, 231, 235
   ota_cache.py3896384%146, 198, 214–217, 235–248, 260, 305–306, 311–312, 328, 384–385, 425–426, 531, 546–550, 684–685, 717–718, 729, 768–773, 786–799, 844, 852–854, 909–912, 916–920, 938–940, 994, 1001–1002, 1004–1005, 1008, 1038–1042
   server_app.py1224365%102–108, 112–121, 173, 175, 181, 191–196, 204, 216–264, 275–277, 283–285
   utils.py28196%31
TOTAL5889119680% 

docs/README.md Outdated
Only the main ECU should have this information.
If this field is not specified, value of `ecu_id` is used as this value.
For compatibility, if this `available_ecu_ids` filed does not exist, the ecu_id of the main ecu is considered the entry for available_ecu_ids.
Copy link
Member

Choose a reason for hiding this comment

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

I think this line should also prefixed with NOTE(or even WARNING), as user might think empty available_ecu_ids means not update any ECUs.
Also, I think this is backward-compatibility?

docs/README.md Outdated
If this field is not specified, value of `ecu_id` is used as this value.
For compatibility, if this `available_ecu_ids` filed does not exist, the ecu_id of the main ecu is considered the entry for available_ecu_ids.

NOTE: The OTA client user (e.g. Agent) has responsibility for filtering out update requests by using this ecu list.
Copy link
Member

@Bodong-Yang Bodong-Yang Jul 31, 2023

Choose a reason for hiding this comment

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

as otaclient can work not only with web.auto agent, but also other implementation that using otaclient. For other implementation, they can use other kinds of implementation to generate the update request. The use of available_ecu_ids is required by web.auto agent, but it is not a requirement to other implementation that uses otaclient.

Copy link
Member

@Bodong-Yang Bodong-Yang Jul 31, 2023

Choose a reason for hiding this comment

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

@obi-t4 After some consideration, I agree that all OTA client user should follow the spec of available_ecu_ids, a.k.a, available_ecu_ids should represent the ECUs to be updated, but the use of available_ecu_ids is not mandatory.

Copy link
Member

@Bodong-Yang Bodong-Yang Jul 31, 2023

Choose a reason for hiding this comment

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

( We can add another line says that alternative client implementation can also use available_ecu_ids yield, but it is not mandatory.

@obi-t4
Copy link
Contributor Author

obi-t4 commented Jul 31, 2023

@Bodong-Yang
update in a00549d, thanks!

@obi-t4 obi-t4 requested a review from Bodong-Yang July 31, 2023 07:45
@Bodong-Yang
Copy link
Member

Bodong-Yang commented Jul 31, 2023

@obi-t4 I have some more questions, please have a check, thank you!

What if some ECUs presented in secondary_ecus field, but not listed in available_ecu_ids, should OTA client include the status of these ECUs in the status response? In other word, should otaclient tracks ECU that listed in secondary_ecus but not in available_ecu_ids?

Another question is, should we expect that secondary_ecus and available_ecu_ids are synced? A.k.a, available_ecu_ids are always either the same as secondary_ecus, or available_ecu_ids==<my_ecu_id> + secondary_ecus?

@obi-t4
Copy link
Contributor Author

obi-t4 commented Aug 1, 2023

@Bodong-Yang

What if some ECUs presented in secondary_ecus field, but not listed in available_ecu_ids, should OTA client include the status of these ECUs in the status response? In other word, should otaclient tracks ECU that listed in secondary_ecus but not in available_ecu_ids?

secondary_ecus and available_ecu_ids are the independent information.
All secondary_ecus responses that can respond should be included in the Status response.
available_ecu_ids is the information for ota-client user(=caller, aka agent).

OTA client doesn't have a ECU filtering policy.

Another question is, should we expect that secondary_ecus and available_ecu_ids are synced? A.k.a, available_ecu_ids are always either the same as secondary_ecus, or available_ecu_ids==<my_ecu_id> + secondary_ecus?

they are the independent information.

Copy link
Member

@Bodong-Yang Bodong-Yang left a comment

Choose a reason for hiding this comment

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

Thank you for the updates!

@obi-t4 obi-t4 merged commit 0c7efdb into main Aug 1, 2023
3 checks passed
@obi-t4 obi-t4 deleted the feat/update-docs branch August 1, 2023 04:46
Bodong-Yang added a commit that referenced this pull request Aug 30, 2023
Commits from v3.5.1 to v3.6.0

* Bug fix
1. [Fix] boot_ctrl.cboot: post_update error handling #240
2. [Fix] otaclient_stub: fix always export self ECU id in available_ecu_ids  #250

* Features
1. [Feature] extend ota-file-cache-control header #244
2. [Feature & Refinement] otaproxy: OTA file sha256 aware, support new header  #245
3. [Feature & Refinement] otaproxy, otaclient: introduce external cache dev support #246

* Chore
1. doc: update document especially available_ecu_ids #248
2. [Chore] tools: add new OTA status monitor #251
3. [Chore] tools: add offline OTA image builder #252

* Deps
1. Bump grpcio from 1.40.0 to 1.53.0 in /tools/emulator #241
2. [Deps] bumps grpcio to 1.53.1 #243
3. Bump aiohttp from 3.8.1 to 3.8.5 in /otaclient #247
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.

2 participants