osmocom-bb/4f36345180c9556086ef8fd4f3d...

313 lines
10 KiB
Plaintext

{
"comments": [
{
"unresolved": true,
"key": {
"uuid": "47506186_d27c1bcf",
"filename": "/COMMIT_MSG",
"patchSetId": 2
},
"lineNbr": 13,
"author": {
"id": 1000010
},
"writtenOn": "2023-01-30T19:56:15Z",
"side": 1,
"message": "The \u0027gsmtap\u0027 prefix in each command looks redundant to me, given that they all belong to \u0027gsmtap\u0027 node.",
"range": {
"startLine": 13,
"startChar": 1,
"endLine": 13,
"endChar": 7
},
"revId": "4f36345180c9556086ef8fd4f3d0bd22af9d2abd",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": true,
"key": {
"uuid": "3ae97f48_a444eef3",
"filename": "/COMMIT_MSG",
"patchSetId": 2
},
"lineNbr": 13,
"author": {
"id": 1000074
},
"writtenOn": "2023-01-30T20:01:33Z",
"side": 1,
"message": "I still prefer the way it looks this way, the commands are more self descriptive and easy to match in other apps too.",
"parentUuid": "47506186_d27c1bcf",
"range": {
"startLine": 13,
"startChar": 1,
"endLine": 13,
"endChar": 7
},
"revId": "4f36345180c9556086ef8fd4f3d0bd22af9d2abd",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": true,
"key": {
"uuid": "7707255a_df939daf",
"filename": "/COMMIT_MSG",
"patchSetId": 2
},
"lineNbr": 13,
"author": {
"id": 1000010
},
"writtenOn": "2023-01-30T20:04:56Z",
"side": 1,
"message": "But there is no dedicated \u0027gsmtap\u0027 node in other apps, so it already does not match. The command style in both osmo-bts (gsmtap-sapi) and osmo-pcu (gsmtap-category) does not match either, you\u0027re just mixing them up here.",
"parentUuid": "3ae97f48_a444eef3",
"range": {
"startLine": 13,
"startChar": 1,
"endLine": 13,
"endChar": 7
},
"revId": "4f36345180c9556086ef8fd4f3d0bd22af9d2abd",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": true,
"key": {
"uuid": "c88183a4_f02fdff1",
"filename": "/COMMIT_MSG",
"patchSetId": 2
},
"lineNbr": 13,
"author": {
"id": 1000074
},
"writtenOn": "2023-01-31T11:04:41Z",
"side": 1,
"message": "I\u0027m not mixing them here, I\u0027m establishing 2 layers of granularity. First based on lchan, and then further specify in the lchan which kind of messages we want (this is important in gprs because there may be lots of DUMMY messages).",
"parentUuid": "7707255a_df939daf",
"range": {
"startLine": 13,
"startChar": 1,
"endLine": 13,
"endChar": 7
},
"revId": "4f36345180c9556086ef8fd4f3d0bd22af9d2abd",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": true,
"key": {
"uuid": "e4590ff6_c3d752c9",
"filename": "/COMMIT_MSG",
"patchSetId": 2
},
"lineNbr": 13,
"author": {
"id": 1000010
},
"writtenOn": "2023-01-31T11:24:48Z",
"side": 1,
"message": "Ok, let\u0027s see what the others think about the command style/consistency.",
"parentUuid": "c88183a4_f02fdff1",
"range": {
"startLine": 13,
"startChar": 1,
"endLine": 13,
"endChar": 7
},
"revId": "4f36345180c9556086ef8fd4f3d0bd22af9d2abd",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": true,
"key": {
"uuid": "221d65ad_3074e131",
"filename": "/COMMIT_MSG",
"patchSetId": 2
},
"lineNbr": 13,
"author": {
"id": 1000074
},
"writtenOn": "2023-01-31T11:36:21Z",
"side": 1,
"message": "Ack",
"parentUuid": "e4590ff6_c3d752c9",
"range": {
"startLine": 13,
"startChar": 1,
"endLine": 13,
"endChar": 7
},
"revId": "4f36345180c9556086ef8fd4f3d0bd22af9d2abd",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": true,
"key": {
"uuid": "5f13da34_ed918a95",
"filename": "/COMMIT_MSG",
"patchSetId": 2
},
"lineNbr": 26,
"author": {
"id": 1000010
},
"writtenOn": "2023-01-30T20:02:26Z",
"side": 1,
"message": "I don\u0027t like the inconsistency between \u0027gsmtap-sapi\u0027 and \u0027gsmtap-category\u0027. My proposal:\n\n gsmtap\n remote-host HOST\n lchan (bcch|ccch|rach|agch|...)\n lchan (enable-all|disable-all)\n pchan pdch (dl-dummy|dl-ctrl|...)\n pchan pdch (enable-all|disable-all)",
"range": {
"startLine": 26,
"startChar": 1,
"endLine": 26,
"endChar": 16
},
"revId": "4f36345180c9556086ef8fd4f3d0bd22af9d2abd",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": true,
"key": {
"uuid": "86c174e5_70ce782d",
"filename": "/COMMIT_MSG",
"patchSetId": 2
},
"lineNbr": 26,
"author": {
"id": 1000074
},
"writtenOn": "2023-01-31T11:04:41Z",
"side": 1,
"message": "I\u0027m not really liking this proposal, it differs too much from separations done in other apps.",
"parentUuid": "5f13da34_ed918a95",
"range": {
"startLine": 26,
"startChar": 1,
"endLine": 26,
"endChar": 16
},
"revId": "4f36345180c9556086ef8fd4f3d0bd22af9d2abd",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": true,
"key": {
"uuid": "8d0298c5_1ce808be",
"filename": "/COMMIT_MSG",
"patchSetId": 2
},
"lineNbr": 26,
"author": {
"id": 1000074
},
"writtenOn": "2023-01-31T11:05:56Z",
"side": 1,
"message": "If you really think I should, I can remove the \"gsmtap-\" prefix from commands (I actually prefer the way it is now).\nOther than that, I don\u0027t see it makes sense to substantialy change namings.",
"parentUuid": "86c174e5_70ce782d",
"range": {
"startLine": 26,
"startChar": 1,
"endLine": 26,
"endChar": 16
},
"revId": "4f36345180c9556086ef8fd4f3d0bd22af9d2abd",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": true,
"key": {
"uuid": "f587d582_2af9ccc2",
"filename": "/COMMIT_MSG",
"patchSetId": 2
},
"lineNbr": 26,
"author": {
"id": 1000004
},
"writtenOn": "2023-01-31T11:52:52Z",
"side": 1,
"message": "tbh, I don\u0027t get the \"sapi\" vs \"category\" difference here. \n\nIMHO, calling it gsmtap-sapi only makes sense if we use the exact same syntax/naming as in osmo-bts, where indeed as fixeria points out it was introduced to match the proprietary DSP PHY intrface \"SAPI\" value.\n\nAnd if we\u0027re not keeping it syntacticall identical to osmo-bts, then there\u0027s no need to call the circuit switched part \"sapi\" and the GPRS side \"category\"?",
"parentUuid": "8d0298c5_1ce808be",
"range": {
"startLine": 26,
"startChar": 1,
"endLine": 26,
"endChar": 16
},
"revId": "4f36345180c9556086ef8fd4f3d0bd22af9d2abd",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": true,
"key": {
"uuid": "aff45344_a90148a2",
"filename": "/COMMIT_MSG",
"patchSetId": 2
},
"lineNbr": 26,
"author": {
"id": 1000074
},
"writtenOn": "2023-01-31T12:04:19Z",
"side": 1,
"message": "There\u0027s several topics here:\n\n* \"sapi\" naming: I can rename this from \"gsmtap-sapi\" to \"gsm-channel\" or \"lchan\" as fixeria suggests, since indeed the \"sapi\" thing seems to come from l1 in sysmobts.\n\n* \"sapi\" vs \"category\":\nIt\u0027s not really CS vs PS we are discussing here. The idea is that this VTY config severes purpose for any layer23 app, which is both CS and PS. The idea here is that in general the \"sapi\" level (aka now \"lchan\", eg \"lchan pdtch\") serves the purpose of enabling/disabling gsmtap sending messages received/sent on that lchan.\n\nFor GPRS, we really want to have finer granularity on higher layer than simply enabling or disabling the entire lchan, and hence it is why we have an extra set of commands to enable/disable which type of messages are logged.\n\n\nSo MY PROPOSAL:\n- (I already updated the code using the enum/value_string from libosmcoore)\n- rename \"gsmtap-sapi\" to \"lchan\" and \"gsmtap-category\" to \"category gprs\". I don\u0027t like the idea of having \"category pdtch\" here because for instance for \"dl-unknown\" one doesn\u0027t really know whether it\u0027s PDTCH or PACCH. In fact the PACCH is all a bit blurry to me, having to check too many levels to find out whether it\u0027s PACCH vs PDTCH.",
"parentUuid": "f587d582_2af9ccc2",
"range": {
"startLine": 26,
"startChar": 1,
"endLine": 26,
"endChar": 16
},
"revId": "4f36345180c9556086ef8fd4f3d0bd22af9d2abd",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": true,
"key": {
"uuid": "f147e573_6bd89028",
"filename": "src/host/layer23/include/osmocom/bb/common/l23_app.h",
"patchSetId": 2
},
"lineNbr": 19,
"author": {
"id": 1000010
},
"writtenOn": "2023-01-30T19:56:15Z",
"side": 1,
"message": "BTS?",
"range": {
"startLine": 19,
"startChar": 7,
"endLine": 19,
"endChar": 10
},
"revId": "4f36345180c9556086ef8fd4f3d0bd22af9d2abd",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": true,
"key": {
"uuid": "c7ee473c_79a94526",
"filename": "src/host/layer23/include/osmocom/bb/common/l23_app.h",
"patchSetId": 2
},
"lineNbr": 19,
"author": {
"id": 1000074
},
"writtenOn": "2023-01-30T20:01:33Z",
"side": 1,
"message": "Ack, I\u0027ll fix",
"parentUuid": "f147e573_6bd89028",
"range": {
"startLine": 19,
"startChar": 7,
"endLine": 19,
"endChar": 10
},
"revId": "4f36345180c9556086ef8fd4f3d0bd22af9d2abd",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
}
]
}