Skip to content

boards:mega2560: add debug and debug-server targets #1696

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 26, 2014
Merged

boards:mega2560: add debug and debug-server targets #1696

merged 1 commit into from
Sep 26, 2014

Conversation

N8Fear
Copy link

@N8Fear N8Fear commented Sep 23, 2014

This PR adds the make tagets debug and debug-server to the Arduino Mega 2560.

Tested in conjunction with the AVR Dragon board as ISP/OCD tool.

@N8Fear N8Fear added Platform: AVR Platform: This PR/issue effects AVR-based platforms Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: build system Area: Build system labels Sep 23, 2014
@LudwigKnuepfer
Copy link
Member

I guess it's fine, cant test, reassigning to @haukepetersen because he invented the debugserver and mentioning @thomaseichinger because he implemented it for another board ;)

@N8Fear
Copy link
Author

N8Fear commented Sep 25, 2014

I've just found an issue myself. I'll push a fix later.

@N8Fear
Copy link
Author

N8Fear commented Sep 25, 2014

The problem was that using ctrl-c inside gdb caused avarice to terminate because it received a SIGINT. I fixed the problem by calling avarice via setsid which removes it from the process group. Works for me this way...

@LudwigKnuepfer
Copy link
Member

OT: Can you check whether this problem might exists with other implementors as well?

@N8Fear
Copy link
Author

N8Fear commented Sep 26, 2014

@LudwigOrtmann: Due to lack of hardware I can't really check it. By looking at the make files and scripts I guess that the implementation of the debug target of the iot-lab_M3 may be affected also (as far as I can tell it uses openocd to provide a debugging server and connects afterwards via arm-none-eabi-gdb to that session). To test it someone with the hardware simply has to make debug for it, continue inside gdb and press ctrl-c afterwards to cause a break in the execution (and return to the gdb prompt). If it's affected it will loose the connection to the server at that moment (due to the server getting killed by SIGINT).
If it's not affected, openocd likely can gracefully handle the SIGINT (which avarice obviously doesn't).
All other boards seem to implement debugging in other ways (even the openocd ones: they seem to run gdb from inside openocd somehow).
At least that's what I get from reading the implementations I found via grep -r DEBUGGER.

Unrelated to that Travis bailed out again... 👎

@LudwigKnuepfer
Copy link
Member

Yeah, that's what I meant by looking .. Thank you!

@LudwigKnuepfer
Copy link
Member

Kicked the baby!

@N8Fear
Copy link
Author

N8Fear commented Sep 26, 2014

One kick and Travis is happy again...

@LudwigKnuepfer
Copy link
Member

You're happy with this PR now, right?

@N8Fear
Copy link
Author

N8Fear commented Sep 26, 2014

It works for me and does what I wanted it to do. So yes: if noone else has any objections - I'm happy...

@thomaseichinger
Copy link
Member

openocd is not effected by using ctrl-c in gdb.

LudwigKnuepfer
LudwigKnuepfer reviewed Sep 26, 2014
View reviewed changes
boards/arduino-mega2560/Makefile.include Outdated
Copy link
Member

Choose a reason for hiding this comment

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

How can this be overridden?

Copy link
Author

Choose a reason for hiding this comment

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

see general comment - sorry...

Copy link
Member

Choose a reason for hiding this comment

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

PROGRAMMER sounds better, yes, but I think the = should be a ?= then, right?

Copy link
Member

Choose a reason for hiding this comment

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

Also, I'd split out the stk500v2 into PROGRAMMER and put the rest into FFLAGS .. ?

Copy link
Member

Choose a reason for hiding this comment

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

.. or PROGRAMMERFLAGS if they are independent.

Copy link
Member

Choose a reason for hiding this comment

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

Anyways - I guess conditional assignments should be preferred if this enables setting the variable in another (e.g. application) Makefile.

Copy link
Author

Choose a reason for hiding this comment

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

So you think I should use conditional assignments for PROGRAMMER (?=)?
Does it make sense to make an application setting the used programmer?

Copy link
Member

Choose a reason for hiding this comment

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

Kind of.

It might be unusual but I've used similar exports sometimes to save having to set up the build environment. Either way I don't see any benefit to unconditionally setting it in this case.

Copy link
Member

Choose a reason for hiding this comment

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

Hm, maybe an example:
I have two devices to flash - node and gateway.
Now, I set PROGRAMMEFLAGS to use device A for the gateway at all times and to use device B for the node at all times. That way, I can have both devices attached at the same time and make sure the correct application is deployed.

This is probably not a Makefile that will be part of RIOT, but very handy for me as a developer.

Copy link
Member

Choose a reason for hiding this comment

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

(Note: conditionally setting not only the PROGRAMMER variable here..)

@N8Fear
Copy link
Author

N8Fear commented Sep 26, 2014

At the command line: make BOARD=arduino-mega2560 flash is the default, for the AVR Dragon it works with make BOARD=arduino-mega2560 FLASH_TOOL=dragon_isp flash.
Now that I think of it, PROGRAMMER may be the better choice of words. Your thoughts?

@N8Fear
Copy link
Author

N8Fear commented Sep 26, 2014

@LudwigOrtmann: it works this way - and it's a little bit more consistent. What do you think?

@N8Fear
Copy link
Author

N8Fear commented Sep 26, 2014

I've squashed the changes.

@LudwigKnuepfer
Copy link
Member

With the exception of the = vs ?= question I'm quite happy.

@N8Fear
Copy link
Author

N8Fear commented Sep 26, 2014

Ok - I've changed the = to ?= for PROGRAMMER.

LudwigKnuepfer
LudwigKnuepfer reviewed Sep 26, 2014
View reviewed changes
boards/arduino-mega2560/Makefile.include
Copy link
Member

Choose a reason for hiding this comment

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

this too?
Compare #1696 (comment)

Copy link
Author

Choose a reason for hiding this comment

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

Since PORT can be (and has to be) overridden anyways if two different boards should be flashed via one Makefile I think, this shouldn't be necessary. I can change it anyways though, if you consider it better...

Copy link
Member

Choose a reason for hiding this comment

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

Oh, you're right, I wasn't looking closely. Pardon!

@LudwigKnuepfer
Copy link
Member

ACK & go

LudwigKnuepfer added a commit that referenced this pull request Sep 26, 2014
@LudwigKnuepfer
Merge pull request #1696 from N8Fear/mega2560-debug
99760f3 
boards:mega2560: add debug and debug-server targets
@LudwigKnuepfer LudwigKnuepfer merged commit 99760f3 into RIOT-OS:master Sep 26, 2014
@N8Fear N8Fear deleted the mega2560-debug branch September 26, 2014 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@haukepetersen haukepetersen

Labels
Area: build system Area: Build system Platform: AVR Platform: This PR/issue effects AVR-based platforms Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@N8Fear @LudwigKnuepfer @thomaseichinger @haukepetersen