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

[Fix] otaclient_stub: fix behavior related available_ecu_ids #249

Closed
wants to merge 4 commits into from

Conversation

Bodong-Yang
Copy link
Member

@Bodong-Yang Bodong-Yang commented Jul 31, 2023

Description

This PR fixes unintended behavior related to the available_ecu_ids field in the status API response, which my ECU id will always being included in the available_ecu_ids. The bug is introduced by #212 .

Cause of the bug

In #212 , ECU status tracker is implemented by _ECUTracker, and all status are stored within ECUStatusStorage inst. The problem is caused by _ECUTracker by default always tracking local my ECU, no matter whether the local my ECU is listed in available_ecu_ids or not. Further, ECUStatusStorage will always include all tracked ECU into available_ecu_ids, along with merging the available_ecu_ids field from ecu_info.yaml, causing the problem.

More about available_ecu_ids

The intention for introducing available_ecu_ids, is to define the set of ECUs that should be engaged in OTA update events. Only ECUs listed in this field should be updated, and should be tracked by main ECU's otaclient/web.auto agent.

For web.auto user, web.auto agent will use available_ecu_ids to generate OTA update list, only ECUs included in available_ecu_ids will be included in the OTA update request.

For backward compatibility, if available_ecu_ids is empty, include my ECU id in the available_ecu_ids list. (check ecu_info.ECUInfo.get_available_ecu_ids method).

Check doc/README.md and doc/SERVICES.md for more details.

Code changes related to Bug fix

The fix is to ensure only ECUs listed in available_ecu_ids will be tracked by otaclient_stub, and status being collected and reported via status API.

  1. otaclient_stub.ECUStatusStorage: using two different attrs to store _tracking_ecus(internal use) and _available_ecu_ids(for used in status response), only update _available_ecu_ids by merging sub ECU's response and ecu_info.yaml to prevent unintended changes to status response.
  2. otaclient_stub.ECUStatusStorage: do not track and store status from child ECUs that doesn't listed in any available_ecu_ids,
  3. otaclient_stub._ECUTracker: when launching ECU trackers for tracked ECU, check against available_ecu_ids list from ecu_info.yaml, only tracking ECUs that listed in available_ecu_ids.

Extra confirmation

  1. ecu_info: confirm the behavior if available_ecu_ids field is empty or not presented in ecu_info.yaml, add my ECU id into the list is implemented.

Bug fix

Current behavior

  1. my ECU id will always being included in the available_ecu_ids in status API response, and my ECU status also always being included in status API response, even my ECU is not listed in available_ecu_ids and not expected to receive OTA update.

Behaivor after fix

  1. my ecu_id(local ECU id) and my ECU status will not be included in the status API response if my ECU is not listed available_ecu_ids and not receiving OTA update.

Related links & ticket

  1. [Feature&refinement] implement new multiple ECU OTA update spec, refactoring otaclient_stub module #212
  2. doc: update document especially available_ecu_ids #248
  3. OTA service API handler design document: document is updated to include new information related to available_ecu_ids.

@Bodong-Yang Bodong-Yang added the bug Something isn't working label Jul 31, 2023
@Bodong-Yang Bodong-Yang self-assigned this Jul 31, 2023
@Bodong-Yang Bodong-Yang requested a review from obi-t4 July 31, 2023 08:20
@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.py3507977%81–90, 99–101, 106, 138, 165, 168, 261, 281, 283, 307, 352, 407, 417, 485–486, 514, 524, 544–561, 565–579, 583–586, 638, 725–734, 766–816, 819
   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
TOTAL5896120080% 

@Bodong-Yang
Copy link
Member Author

Closed as new information acquired, and the fix should be re-considered and re-implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant