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

Rn2483 driver In Progress, Configuration Mostly Complete #2

Open
wants to merge 27 commits into
base: master
Choose a base branch
from

Conversation

JustinDMorrison
Copy link

Added ability to set board configuration values by sending "radio set " signals to board.

Note: Please adhere to Contributing Guidelines.

Summary

Update this section with information on why change is necessary,
what it exactly does and how, if new feature shows up, provide
references (dependencies, similar problems and solutions), etc.

Impact

Update this section, where applicable, on how change affects users,
build process, hardware, documentation, security, compatibility, etc.

Testing

Update this section with details on how did you verify the change,
what Host was used for build (OS, CPU, compiler, ..), what Target was
used for verification (arch, board:config, ..), etc. Providing build
and runtime logs from before and after change is highly appreciated.

linguini1 and others added 5 commits October 17, 2024 14:18
…ice. Implemented with the rn2483_radio_set_cmd function which uses an enum to represent each cmd and sends a radio set <message> to set the desired values of the corresponding cmd. void *data is the generic pointer which will be set to different data types, so appropriate pointer casting was used. All inputs to the radio_set_cmd function are presumed to have already been checked, which is why there are no checks in the function itself. A dummy file_read call is a temporary fix to get rid of an annoying bug where the 'sys get ver' value would be the first values in the read buffer after startup. Logging is used mostly for debugging at this point through syslog.
Copy link

github-actions bot commented Nov 5, 2024

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

@linguini1 linguini1 left a comment

Choose a reason for hiding this comment

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

Please revert the auto-formatting on the rp2040_common_bringup.c file.
Also revert the commit of the w4349A0_7_95_49_00_combined.h since that is a firmware WiFi firmware file that is not part of the code here.

Great work so far!

drivers/wireless/lpwan/rn2483/rn2483.c Outdated Show resolved Hide resolved
drivers/wireless/lpwan/rn2483/rn2483.c Show resolved Hide resolved
drivers/wireless/lpwan/rn2483/rn2483.c Outdated Show resolved Hide resolved
drivers/wireless/lpwan/rn2483/rn2483.c Outdated Show resolved Hide resolved
drivers/wireless/lpwan/rn2483/rn2483.c Outdated Show resolved Hide resolved
drivers/wireless/lpwan/rn2483/rn2483.c Outdated Show resolved Hide resolved
drivers/wireless/lpwan/rn2483/rn2483.c Outdated Show resolved Hide resolved
Copy link

@WhyNot180 WhyNot180 left a comment

Choose a reason for hiding this comment

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

Just wanted to recommend that you keep your commit messages curt (something like 2 sentences) just to make it easier to figure out it changes. You don't have to explain what's going on in the message, but rather do so instead in code comments (if things aren't self-evident). Another thing is to not have all your changes in one commit, a commit should only do one thing (e.g. refactor code, add specific functionality etc.). Otherwise, excluding @linguini1's comments, LGTM.

@WhyNot180
Copy link

Just wanted to recommend that you keep your commit messages curt (something like 2 sentences) just to make it easier to figure out it changes. You don't have to explain what's going on in the message, but rather do so instead in code comments (if things aren't self-evident). Another thing is to not have all your changes in one commit, a commit should only do one thing (e.g. refactor code, add specific functionality etc.). Otherwise, excluding @linguini1's comments, LGTM.

Oops, looks like NuttX's contributing guidelines tell you to make your commits messages like that so it puts it in the pull request, my mistake. It would still be a good idea to separate out your commits, however.

@linguini1
Copy link

Just wanted to recommend that you keep your commit messages curt (something like 2 sentences) just to make it easier to figure out it changes. You don't have to explain what's going on in the message, but rather do so instead in code comments (if things aren't self-evident). Another thing is to not have all your changes in one commit, a commit should only do one thing (e.g. refactor code, add specific functionality etc.). Otherwise, excluding @linguini1's comments, LGTM.

Oops, looks like NuttX's contributing guidelines tell you to make your commits messages like that so it puts it in the pull request, my mistake. It would still be a good idea to separate out your commits, however.

Yeah we can keep them granular on the fork here, I usually just squash them all before sending them to NuttX upstream.

Copy link

github-actions bot commented Nov 9, 2024

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

github-actions bot commented Nov 9, 2024

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

…te functions. Removed all enums related to rn2483_radio_set_cmd.
Copy link

github-actions bot commented Nov 9, 2024

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

…nger than required in case we need to adda null terminating character.
Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

…ed define for dummy read buffer length. Would like to get rid of dummy read eventually.
Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

… recommended. I am also subtracting one to keep space for the null terminating character.
Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

@linguini1 linguini1 left a comment

Choose a reason for hiding this comment

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

Outside of review comments, logic all looks good to me! You can start writing your test case for the ioctl calls, or help Jason with the TX implementation. We can merge in RX after review comments are resolved. Looking like an upstream PR might be ready before end of November! Great work!

drivers/wireless/lpwan/rn2483/rn2483.c Outdated Show resolved Hide resolved
drivers/wireless/lpwan/rn2483/rn2483.c Outdated Show resolved Hide resolved
drivers/wireless/lpwan/rn2483/rn2483.c Outdated Show resolved Hide resolved
…t, set priv->config options after sending the radio command instead of before, and now check for the error return value from that radio command call before setting the config's parameters.
Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

@linguini1 linguini1 left a comment

Choose a reason for hiding this comment

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

Great work! I think this is solid logic for the configuration settings, and I doubt you'll find any issues with your ioctl tests. I'll merge in the receive logic at some point soon and then you can write some code to test receiving too (it's quite fragile). I'll get you the COTS board and last year's test scripts to try sending actual radio signals!

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

bskdany pushed a commit that referenced this pull request Jan 3, 2025
test code hello_main

int main(int argc, FAR char *argv[])
{
  uint32_t *p = 0xdeedbeff;
  *p = 0xffffff;

  printf("%p\n %x\n", p, *p);
  return 0;
}

qemu mps3-an547 hello_main :
Triggering an exception, and gdb backtrace is:

before:
(gdb) bt
/#0  0x0001168a in systick_getstatus (lower_=0x100010c <g_systick_lower>, status=0x1000a30 <g_intstackalloc+1600>)
    at /home/ajh/work/vela_system/nuttx/arch/arm/src/armv8-m/arm_systick.c:142
/#1  0x000122f4 in current_usec () at /home/ajh/work/vela_system/nuttx/drivers/timers/arch_timer.c:105
/#2  0x0001234c in udelay_accurate (microseconds=250000) at /home/ajh/work/vela_system/nuttx/drivers/timers/arch_timer.c:115
/#3  0x000124bc in up_udelay (microseconds=250000) at /home/ajh/work/vela_system/nuttx/drivers/timers/arch_timer.c:463
/#4  0x0001249e in up_mdelay (milliseconds=250) at /home/ajh/work/vela_system/nuttx/drivers/timers/arch_timer.c:446
/#5  0x0000920c in reset_board () at /home/ajh/work/vela_system/nuttx/sched/misc/assert.c:830
/apache#6  0x0000937c in _assert (filename=0x393f8 "/arch/arm/src/armv8-m/arm_busfault.c", linenum=113, msg=0x393f0 "panic", regs=0x1008500)
    at /home/ajh/work/vela_system/nuttx/sched/misc/assert.c:940
/apache#7  0x00000e2c in arm_busfault (irq=3, context=0x1008500, arg=0x0 <up_ndelay>) at /home/ajh/work/vela_system/nuttx/arch/arm/src/armv8-m/arm_busfault.c:113
/apache#8  0x000012d2 in arm_hardfault (irq=3, context=0x1008500, arg=0x0 <up_ndelay>)
    at /home/ajh/work/vela_system/nuttx/arch/arm/src/armv8-m/arm_hardfault.c:142
/apache#9  0x00008b20 in irq_dispatch (irq=3, context=0x1008500) at /home/ajh/work/vela_system/nuttx/sched/irq/irq_dispatch.c:145
/apache#10 0x0000041a in arm_doirq (irq=3, regs=0x1008500) at /home/ajh/work/vela_system/nuttx/arch/arm/src/armv8-m/arm_doirq.c:103
/apache#11 0x0000034e in exception_common () at /home/ajh/work/vela_system/nuttx/arch/arm/src/armv8-m/arm_exception.S:224

after:
(gdb) bt
/#0  systick_is_running () at /home/ajh/work/vela_system/nuttx/arch/arm/src/armv8-m/arm_systick.c:106
/#1  0x000125c0 in systick_getstatus (lower_=0x1000114 <g_systick_lower>, status=0x1007a20)
    at /home/ajh/work/vela_system/nuttx/arch/arm/src/armv8-m/arm_systick.c:141
/#2  0x0001323c in current_usec () at /home/ajh/work/vela_system/nuttx/drivers/timers/arch_timer.c:105
/#3  0x00013294 in udelay_accurate (microseconds=250000) at /home/ajh/work/vela_system/nuttx/drivers/timers/arch_timer.c:115
/#4  0x00013404 in up_udelay (microseconds=250000) at /home/ajh/work/vela_system/nuttx/drivers/timers/arch_timer.c:463
/#5  0x000133e6 in up_mdelay (milliseconds=250) at /home/ajh/work/vela_system/nuttx/drivers/timers/arch_timer.c:446
/apache#6  0x00008c5c in reset_board () at /home/ajh/work/vela_system/nuttx/sched/misc/assert.c:816
/apache#7  0x00008e88 in _assert (filename=0x39408 "/arch/arm/src/armv8-m/arm_busfault.c", linenum=113, msg=0x39400 "panic", regs=0x1007cf0)
    at /home/ajh/work/vela_system/nuttx/sched/misc/assert.c:915
/apache#8  0x00000ce4 in arm_busfault (irq=3, context=0x1007cf0, arg=0x0 <up_ndelay>)
    at /home/ajh/work/vela_system/nuttx/arch/arm/src/armv8-m/arm_busfault.c:113
/apache#9  0x0000118a in arm_hardfault (irq=3, context=0x1007cf0, arg=0x0 <up_ndelay>)
    at /home/ajh/work/vela_system/nuttx/arch/arm/src/armv8-m/arm_hardfault.c:142
/apache#10 0x000086cc in irq_dispatch (irq=3, context=0x1007cf0) at /home/ajh/work/vela_system/nuttx/sched/irq/irq_dispatch.c:145
/apache#11 0x0000041e in arm_doirq (irq=3, regs=0x1007cf0) at /home/ajh/work/vela_system/nuttx/arch/arm/src/armv8-m/arm_doirq.c:99
/apache#12 0x00000360 in exception_common () at /home/ajh/work/vela_system/nuttx/arch/arm/src/armv8-m/arm_exception.S:230
/apache#13 0x00027a8c in hello_main (argc=1, argv=0x1006e20) at /home/ajh/work/vela_system/apps/examples/hello/hello_main.c:39
/apache#14 0x00014968 in nxtask_startup (entrypt=0x27a7d <hello_main>, argc=1, argv=0x1006e20)
    at /home/ajh/work/vela_system/nuttx/libs/libc/sched/task_startup.c:72
/apache#15 0x0000f450 in nxtask_start () at /home/ajh/work/vela_system/nuttx/sched/task/task_start.c:116
/apache#16 0x00000000 in ?? ()
(gdb)

qemu armv7a nsh, hello_main:

before:
(gdb) bt
/#0  udelay_coarse (microseconds=156000) at /home/ajh/work/vela_system/nuttx/drivers/timers/arch_alarm.c:67
/#1  up_ndelay (nanoseconds=nanoseconds@entry=250000000) at /home/ajh/work/vela_system/nuttx/drivers/timers/arch_alarm.c:431
/#2  0x0060c630 in up_udelay (microseconds=microseconds@entry=250000) at /home/ajh/work/vela_system/nuttx/drivers/timers/arch_alarm.c:416
/#3  0x0060c644 in up_mdelay (milliseconds=milliseconds@entry=250) at /home/ajh/work/vela_system/nuttx/drivers/timers/arch_alarm.c:401
/#4  0x006056bc in reset_board () at /home/ajh/work/vela_system/nuttx/sched/misc/assert.c:816
/#5  _assert (filename=filename@entry=0x63047f "/arch/arm/src/armv7-a/arm_dataabort.c", linenum=linenum@entry=157, msg=msg@entry=0x62f56d "panic",
    regs=<optimized out>, regs@entry=0x4020af10) at /home/ajh/work/vela_system/nuttx/sched/misc/assert.c:915
/apache#6  0x0060bd74 in arm_dataabort (regs=0x4020af10, dfar=<optimized out>, dfsr=<optimized out>)
    at /home/ajh/work/vela_system/nuttx/arch/arm/src/armv7-a/arm_dataabort.c:157
/apache#7  0x0060bc04 in arm_vectordata () at /home/ajh/work/vela_system/nuttx/arch/arm/src/armv7-a/arm_vectors.S:438
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
(gdb)

after:
(gdb) bt
/#0  udelay_coarse (microseconds=192000) at /home/ajh/work/vela_system/nuttx/drivers/timers/arch_alarm.c:67
/#1  up_ndelay (nanoseconds=nanoseconds@entry=250000000) at /home/ajh/work/vela_system/nuttx/drivers/timers/arch_alarm.c:431
/#2  0x0060c650 in up_udelay (microseconds=microseconds@entry=250000) at /home/ajh/work/vela_system/nuttx/drivers/timers/arch_alarm.c:416
/#3  0x0060c664 in up_mdelay (milliseconds=milliseconds@entry=250) at /home/ajh/work/vela_system/nuttx/drivers/timers/arch_alarm.c:401
/#4  0x006056bc in reset_board () at /home/ajh/work/vela_system/nuttx/sched/misc/assert.c:816
/#5  _assert (filename=filename@entry=0x63047f "/arch/arm/src/armv7-a/arm_dataabort.c", linenum=linenum@entry=157, msg=msg@entry=0x62f56d "panic",
    regs=<optimized out>, regs@entry=0x4020af10) at /home/ajh/work/vela_system/nuttx/sched/misc/assert.c:915
/apache#6  0x0060bd94 in arm_dataabort (regs=0x4020af10, dfar=<optimized out>, dfsr=<optimized out>)
    at /home/ajh/work/vela_system/nuttx/arch/arm/src/armv7-a/arm_dataabort.c:157
/apache#7  0x0060bc08 in arm_vectordata () at /home/ajh/work/vela_system/nuttx/arch/arm/src/armv7-a/arm_vectors.S:453
/apache#8  0x00620cd4 in hello_main (argc=4999, argv=0x0) at /home/ajh/work/vela_system/apps/examples/hello/hello_main.c:41
/apache#9  0x0060d320 in nxtask_startup (entrypt=0x620cc4 <hello_main>, argc=1, argv=0x4020a088)
    at /home/ajh/work/vela_system/nuttx/libs/libc/sched/task_startup.c:72
/apache#10 0x00609b50 in nxtask_start () at /home/ajh/work/vela_system/nuttx/sched/task/task_start.c:116
/apache#11 0x00000000 in ?? ()

qemu risc-v nsh

before:

(gdb) bt
/#0  udelay_coarse (microseconds=228000, microseconds@entry=891896832) at timers/arch_alarm.c:67
/#1  up_ndelay (nanoseconds=nanoseconds@entry=250000000) at timers/arch_alarm.c:431
/#2  0x8000397e in up_udelay (microseconds=microseconds@entry=250000) at timers/arch_alarm.c:416
/#3  0x80003988 in up_mdelay (milliseconds=milliseconds@entry=250) at timers/arch_alarm.c:401
/#4  0x80011f1c in reset_board () at misc/assert.c:813
/#5  0x80011f7a in _assert (filename=filename@entry=0x0, linenum=linenum@entry=0, msg=msg@entry=0x8002114c "panic", regs=<optimized out>,
    regs@entry=0x80030704) at misc/assert.c:915
/apache#6  0x80006ad6 in riscv_exception (mcause=<optimized out>, regs=0x80030704, args=<optimized out>) at common/riscv_exception.c:129
/apache#7  0x80000d9e in riscv_doirq (irq=7, regs=<optimized out>) at common/riscv_doirq.c:99
/apache#8  0x80000164 in exception_common () at common/riscv_exception_common.S:210
Backtrace stopped: frame did not save the PC
(gdb)

after

(gdb) bt
/#0  0x80003922 in udelay_coarse (microseconds=90000, microseconds@entry=891896832) at timers/arch_alarm.c:67
/#1  up_ndelay (nanoseconds=nanoseconds@entry=250000000) at timers/arch_alarm.c:431
/#2  0x8000397e in up_udelay (microseconds=microseconds@entry=250000) at timers/arch_alarm.c:416
/#3  0x80003988 in up_mdelay (milliseconds=milliseconds@entry=250) at timers/arch_alarm.c:401
/#4  0x80011f2a in reset_board () at misc/assert.c:816
/#5  0x80011f7a in _assert (filename=filename@entry=0x0, linenum=linenum@entry=0, msg=msg@entry=0x8002114c "panic", regs=<optimized out>,
    regs@entry=0x80030704) at misc/assert.c:915
/apache#6  0x80006ad6 in riscv_exception (mcause=<optimized out>, regs=0x80030704, args=<optimized out>) at common/riscv_exception.c:129
/apache#7  0x80000d9e in riscv_doirq (irq=7, regs=<optimized out>) at common/riscv_doirq.c:99
/apache#8  0x80000166 in exception_common () at common/riscv_exception_common.S:215
/apache#9  0x8001792a in hello_main (argc=<optimized out>, argv=<optimized out>) at hello_main.c:41
/apache#10 0x80004b52 in nxtask_startup (entrypt=0x80030704, argc=1, argv=0x800300e8) at sched/task_startup.c:72
/apache#11 0x80001e72 in nxtask_start () at task/task_start.c:116
/apache#12 0x00000000 in ?? ()
Backtrace stopped: frame did not save the PC
(gdb)

Signed-off-by: anjiahao <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants