RoboTutorLLC / RoboTutor_2020

Other
3 stars 1 forks source link

Log config name and config.json #35

Closed kapilv19 closed 3 years ago

kapilv19 commented 3 years ago

Fixes #33

sansyrox commented 3 years ago

Hi @kapilv19 , this looks good.

Attaching the apk for the other devs to test it out as well: https://github.com/RoboTutorLLC/RoboTutor_2020/blob/apk/RoboTutor-log_config-2021-05-11.apk

sansyrox commented 3 years ago

@iamsh4shank , we will squash while merging. Since I can see merge commits in the commit logs, it will be a little difficult to do a manual rebase.

JackMostow commented 3 years ago

@kapilv19 -

  1. Sorry I didn't catch it before, but please output the configuration as a valid JSON expression of the form { "configuration": [...] }, where ... is a JSON array of comma-separated "attribute":"value" pairs rather than a newline-separated array of attribute - value pairs, so that it can be parsed the same way as the rest of the log.
  2. The value of "config_version" used to be specified manually in config.json by concatenating the first character(s) of each configuration variable, which was too easy to forget to do correctly. So now there is code to compute it from the other configuration variables in order to ensure that it's consistent with them. I don't know why the code isn't being called. Also, note that the screenshot shows a JSON syntax error in the last line, namely the superfluous ":" in config_version - ":""}}.
kapilv19 commented 3 years ago

@JackMostow -

  1. Sure, I will change it to a json format.
  2. The code is being called and hasn't been touched by me. I will check why it is returning an empty string.
  3. The json error is due to the format of config only. That error won't come after changing it to a json format

Thank you.

sansyrox commented 3 years ago

Hi @kapilv19 ,

Please ensure that this PR(https://github.com/RoboTutorLLC/RoboTutor_2020/pull/10) before merging the current PR otherwise there will be some inconsistencies in the logging.

JackMostow commented 3 years ago

@kapilv19: RoboTutor-log_config-2021-05-14.apk (at least my copy) apparently ignores config.json and uses the default configuration instead.

download/config.json enables, e.g., show_debug_launcher: { "config_version": "ftttfNULLfCD2ftf", "language_override": false, "show_tutorversion": true, "show_debug_launcher": true, "language_switcher": true, "no_asr_apps": false, "language_feature_id": "LANG_NULL", "show_demo_vids": false, "menu_type": "CD2", "show_helper_button": false, "record_screen_video": true, "include_audio_output_in_screen_video": false "baseDirectory": "roboscreen" }

but RoboTutor__3.4.2.1_000000_2021.05.18.15.03.12_6111001342.json does not:

{"RT_log_version":"1.0.1","RT_log_data":[{"class":"VERBOSE","tag":"RTag","type":"TimeStamp","datetime":"05/18/2021 15:03:12","time":"1621364592865","data":{"RoboTutor":"SessionStart"}}, {"type":"LOG_DATA","tutor":"","class":"INFO","tag":"RTag","time":"1621364592865","data":{"EngineVersion":"3.4.2.1"}}, {"type":"LOG_DATA","tutor":"","class":"INFO","tag":"ROBOTUTOR_CONFIGURATION","time":"1621364592865","data":{"no_asr_apps":"false","include_audio_output_in_screen_video":"false","show_demo_vids":"true","language_switcher":"false","language_override":"true","show_helper_button":"false","record_screen_video":"true","base_directory":"roboscreen","use_placement":"true","record_audio":"false","language_feature_id":"LANG_SW","pinning_mode":"false","menu_type":"CD1","show_tutor_version":"true","show_debug_launcher":"false","config_version":""}},

Please fix ASAP. Thanks!

kapilv19 commented 3 years ago

Try this config.json: { "config_version": "ftttfNULLfCD2ftf", "language_override": false, "show_tutorversion": true, "show_debug_launcher": true, "language_switcher": true, "no_asr_apps": false, "language_feature_id": "LANG_NULL", "show_demo_vids": false, "menu_type": "CD2", "show_helper_button": false, "record_screen_video": true, "include_audio_output_in_screen_video": false "baseDirectory": "roboscreen" }

This config.json is invalid, as there should a comma after "include_audio_output_in_screen_video": false line. @JackMostow sir, please change it accordinlgy and let me know if it works for you afterwards. Thank you.

JackMostow commented 3 years ago

How embarrassing that I unwittingly introduced the syntax error by omitting a comma after "include_audio_output_in_screen_video": falsewhen I edited config.json! Thanks @Kapil Verma for fixing it.

JackMostow commented 3 years ago

Resulting apk will log configuration and record activity screen videos.

JackMostow commented 3 years ago

@kapilv19 - can you please merge the pull request? I don't know which type to do (merge commit, squash, or rebase). Thanks!